Friday, 13 November 2009

Don't wait for perfection

Way back in July I was thinking of writing a post about the new branch listings in Launchpad. I was working on making branch listings for distributions and distroseries work, for example Package branches for Ubuntu and Packages branches for Ubuntu Lucid. But the code wasn't entirely optimised. Then as things happen, the optimisation got pushed back... and back. And finally when I did get the optimisation in, I didn't feel it was worthy of talking about.

I guess the thing to remember is: don't wait for perfection. Sure it wasn't perfect, but if more people were accessing the pages, the optimisation may have happened sooner.

One thing going on at the moment is more integration of the lazr-js widgets. The main merge proposal page now has in in-page multi-line editor for the commit message. Sure, it needs tweaking, but the main functionality is there. More ajaxy goodness is finding its way into Launchpad.

One of the things that I'm thinking about right now is splitting the concepts of code reviews and merge proposals. At the moment we almost use the term interchangeably, which does cause some confusion. I'd like to have the merge proposal reflect the meta-data and information around the intent to have work from one branch be landed or merged into another branch (normally the trunk branch), and the code review the conversation that goes on around the merge proposal. Merge proposals may have an associated code review, but right now, a code review must be associated with a merge proposal.

Associated with this, I'd like to extract some state information. Currently merge proposals have only one status, which really reflects two things. I'd like to break this out into two: review status; and merge status. Review status would be one of: work in progress; needs review; approved; rejected; superseded; and maybe withdrawn. Merge status would be one of: proposed; merged; queued; and merge failed. Queued relates to the merge queues which are currently partially implemented in the Launchpad codebase, and merge failed is the state that a proposal would be set to when a landing robot like PQM or Tarmac attempt to land the branch but it fails due to either merge conflicts or test failures.

My goal for the next six months it to write more often, talk about ideas more, and not wait for perfection.


AmanicA said...

This sounds interesting to me. With Bazaar moving from BundleBuggy to launchpad merge-proposals & code-reviews, this is something I noticed:
Having the review-process tightly linked to the merge-proposals causes the review-thread to become quite disjointed when resubmitting your branch. Hopefully this line of thinking would improve this so it can be easier to follow review comments back to comments made on previous proposals in the same review thread (which is needed for context, like to understand why the branch was re-subitted in the first place).

Tim Penhey (thumper) said...

Well, there are two slightly related changes that have either happened or happening.

Firstly the diffs shown with the code review is updated when the branch is pushed to. One change I'd like to add here is extra notification to the subscribers that the branch has changed.

The second is showing the conversation of the superseded proposals on the subsequent proposals, but not allowing replies to the comments. New comments get added to the current review. We'll probably have some extra indication (like background colour/footer info) on the older comments.