Code review comment for lp:~shakaran/userwebkit/class-refactor

Revision history for this message
Jason Gerard DeRose (jderose) wrote :

Sorry Angel, I'm gonna say no on this one :)

As userwebkit.py is less than 500 lines, I don't think this split is yet needed for maintainability. Plus, I think we need more work on the existing API before we know how such a split should be done.

The startup time of a Python app also has a somewhat steep per-module overhead. So in terms of looking at performance and memory usage, it's generally more import to split modules based on what those modules themselves then import. And in this case, everything needs to import `gi.repository.Gtk` (which is where the biggest overhead is), so if you're using anything from userwebkit.py, there is little benefit to importing just a fragment of it.

One more note: once a split like this is done, the best way to do it is to bzr mv the current userwebkit.py into userwebkit/__init__.py and put new modules inside the userwebkit/ Python package.

I'm going to do the above when I bring back the JavaScript unit tester that used to be in Dmedia (it hasn't yet been ported to Python3 because of dependency issues). But this is something that will only be used for unit tests, not at run-time, so having it in a separate module makes sense.

review: Disapprove

« Back to merge proposal