Split out dbus code from daemon code

Bug #947267 reported by Duncan McGreggor
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu Accomplishments System
Fix Released
Duncan McGreggor

Bug Description

The daemon class AccomplishmentsService is a dbus service object. Even though it has methods that call into a Twisted API, AccomplishmentsService itself is not twisted and should be separated from that code. It should go with other dbusapi code.

Related branches

Revision history for this message
Jono Bacon (jonobacon) wrote :

Is there any particular reason why it would be bad to leave a non-Twisted class (AccomplishmentsService) in with the Twisted one (Accomplishments)?

Changed in ubuntu-accomplishments-system:
status: New → Incomplete
importance: Undecided → Low
Revision history for this message
Duncan McGreggor (oubiwann) wrote :

So, in general, the Python community (in particular, those well-established in the language like the author, Guido, Alex Martelli, etc.) embrace simple Python code and encourage this in developers who create projects with the language.

What does "simple" mean?

 * too much indented code is frowned upon: it's hard to maintain and read, instead break it into logical bits
 * don't throw all code into a single file - split things out into different modules or subpackages
 * don't throw all your code into a single class; break it up by area of concern, logical responsibility, etc.
 * write smaller functions/methods, and more of them

All of these boil down to this: make sure your code is not doing to much. If it is, it's going to be hard for new-commers to understand and adopt, help maintain, etc. It's going to make code merges more challenging, as there will be a greater probability of collisions due to everything being in one files vs. several.

So, to answer your question in particular: we want newcomers to know *exactly* where the Twisted code is, and know where it's called. This will be easier to do if we split it out better. We'll also gain in the process, regarding to the points made earlier in this comment :-)

Revision history for this message
Jono Bacon (jonobacon) wrote :

This makes perfect sense to me. I am a big fan of simplifying the code so it is easy to find where issues are happening. I am just conscious to not trade the simplicity of lots of small methods with a complex tapestry of modules with tiny bits of code in them.

It sounds though like you are advocating the idea of clear logical pieces that we break the API up into (e.g. ImageAPI, ConfigAPI etc) and a consistent programming style (PEP-8).

Changed in ubuntu-accomplishments-system:
status: Incomplete → Confirmed
Changed in ubuntu-accomplishments-system:
assignee: nobody → Duncan McGreggor (oubiwann)
status: Confirmed → In Progress
Jono Bacon (jonobacon)
Changed in ubuntu-accomplishments-system:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.