Code review comment for lp:~sil/desktopcouch/glib-callback-for-changes

Revision history for this message
Eric Casteleijn (thisfred) wrote :

> You'll note that the glib callback function imports the Python gir stuff whe
> nit's called, not at the top level in the module, precisely so that using gir
> is not a dependency *unless you use glib_callback_for_changes*. The apt
> dependency system isn't set up all that well for this; putting this one
> function in a package all by itself would be daft. I suggest that the
> resulting package Suggests python-gir-soup as per http://www.debian.org/doc
> /debian-policy/ch-relationships.html#s-binarydeps.

Yeah, I really hate having a function that will break as soon as you call it, though. I don't suggest having this in a separate package, but having it as a top level function won't show it in introspection of CouchDatabase, and allows us to move it into desktopcouch.application, where I think the dependency (or suggestion) is less of a problem.

> The function delves into interior details of CouchDatabase in order to compute
> a _changes URL. Something external to CouchDatabase should not be using this
> sort of internal API; it's an implementation detail which we may change.

If so we can reimplement the function, I don't consider it as big a problem as having a broken method on our most central class.

> You're right about the pylint stuff, although I don't understand why I didn't
> get told the same things when running the tests? What am I doing wrong there?

The preferred way to run the tests now is ./runtests.py which takes care of the python path and everything else. It will run lint on the changed files only unless there are no local changes, and then it will lint every file that was changed in the branch, so if you want to lint the whole branch, make sure all your changes are committed.

« Back to merge proposal