Code review comment for lp://staging/~bryce/launchpad/lp-617699-api

Revision history for this message
Gavin Panella (allenap) wrote :

Hi Bryce,

This was a bit of a 'mare :) Take a look at a bundle I prepared for
you:

  http://paste.ubuntu.com/512660/

It doesn't fix all the tests, but it gets you to a point where you can
probably fix the rest. As in, there are fewer wtf exceptions.

I'll run you through the changes I've made:

lib/canonical/launchpad/webapp/batching.py

  Adapter for some Storm classes that the web service was tripping
  over. Once I had the navigation stuff (later) done I was getting
  errors about not being able to adapt to IFiniteSequence. It seems
  that an adapter was only ever created for SQLObject joins.

lib/canonical/launchpad/webapp/configure.zcml

  Registration of the above adapter.

lib/lp/bugs/browser/bugtracker.py

  Added a navigation step for the bug tracker. I chose the following
  URL pattern, but it's easy to change:

    /bugs/bugtrackers/
      <bug-tracker-name>/
        +components/<component-group-name>/<component-name>

  This meant I also had to create a navigation class for component
  groups.

lib/lp/bugs/browser/configure.zcml

  Registration of the new navigation class, and canonical URL
  configuration for components and component groups.

lib/lp/bugs/configure.zcml

  The component_group attribute needed to be accessible.

lib/lp/bugs/interfaces/bugtracker.py

  Add component_group to the interface definition and fix the
  definition of the components attribute. On reflection, the
  component_group attribute doesn't actually need to be exported
  (though it does need to be in the interface).

lib/lp/bugs/tests/test_bugtracker_components.py

  The addition of lots of commits seems to make the tests work, or
  fail in a way that makes more sense.

lib/lp/testing/factory.py

  The factory methods should not be using hard-coded strings, so I
  fixed that.

And, yes, it's crazy :) There's a lot of stuff to do to get something
published. Fortunately it is actually worth it in the long run, well,
in my opinion.

Gavin.

« Back to merge proposal