Code review comment for lp:~thumper/launchpad/fix-browser-zcml

Revision history for this message
Paul Hummer (rockstar) wrote :

<mwhudson> 7 >>> canonical_url(getUtility(IBazaarApplication))
<mwhudson> 8 - u'http://launchpad.dev/+code'
<mwhudson> 9 + u'http://code.launchpad.dev/+code'
<mwhudson> the +code there is freaking odd really
<mwhudson> i wonder if it would actually be better to have the root objects be different for each publication
<thumper> mwhudson: probably
<thumper> mwhudson: but somewhat out of scope for this change
<rockstar> thumper, holy balls that's a big diff...
<mwhudson> thumper: yes
<thumper> rockstar: but boring
<rockstar> thumper, might I make a suggestion for the next time?
<thumper> yes
<thumper> rockstar: don't do them all at once?
<rockstar> thumper, yes, for the love of Odin, yes.
<thumper> :)
<rockstar> thumper, here's an example: line 1640-1641 of the diff doesn't look to be along the same lines as the rest of the changes in the branch.
<rockstar> Actually, 1635-1665...
<thumper> rockstar: :)
<thumper> yes, I removed a chunk of code that was showing bad data
<thumper> the person that wrote it thought it was showing the number of new revisions this month for that branch
<thumper> but what it was showing was the number of revisions on all branches this month
<rockstar> thumper, the change makes sense. I just think it would be better to not get lumped in with something that's supposed to be just technical debt.
<rockstar> Is there a bug filed for that change?
<thumper> no
<rockstar> thumper, I think there should be.
* thumper nods

review: Approve

« Back to merge proposal