Code review comment for lp:~edgar-b-dsouza/acire/snippet_dependency_res_1

Revision history for this message
Ed S (edgar-b-dsouza) wrote :

Hi, Jono.

The checking is hooked into bin/acire in the snippet_selected() function, just below your new docs_box code - search for:
"check_results = utils.check_script"

See it working:
a) Module with installation candidate package:
E.g. python-clutter (if installed on your system, you could temporarily uninstall it to test?). When you visit a Clutter snippet, SnippetdependenciesDialog should pop up and prompt you to install it.

b) Module with no installation candidate package:
In my case, that was Zeitgeist. If you have a PPA for Zeitgeist, and can temp-uninstall the package, temp-remove the PPA, and restart Acire to refresh the list of available packages - and then visit a Zeitgeist snippet. The Info Page button should take you to (which I put up yesterday, please read/edit it?).

elegant way of exposing this in the UI:
I like your idea. I'd thought of notifying in statusbar, but that was too unobtrusive. Your yellow bar is hard for the user to miss. I'll look for a GTK control to use, and will propose a subsequent feature branch merge.

third-party PPAs for modules out of scope for a first cut:
Yes, I agree :-) that's why I called it a first cut :-). Like I asked in question #104601, (for a later feature branch), I need some pointers/ideas on how to go about that. OTOH, if you think the Wiki page I mentioned is a viable stopgap for some time, that's cool too.

as pluggable as possible for different distros:
Yes, I'd thought of that but wanted to first focus on feature addition; I find it a little complex to simultaneously add in such abstractions. If you like, I can work on that earlier (let me know) to:
- move the distro-specific functions into separate module scripts
- add distro-sniff code in the generic ''
- use the distro-sniff to import the relevant pkg-handling module script

Happy to put in the work :-) and am hoping I may feature in authors/Help > About at some point...


« Back to merge proposal