Sounds right to me!

> From: Robert Lupton <[EMAIL PROTECTED]>
> Reply-To: LSST Data Management <[email protected]>
> Date: Thu, 30 Nov 2006 12:35:13 -0500
> To: LSST Data Management <[email protected]>
> Subject: Re: [LSST-data] DataChallenge: Code reviews
> 
> We're converging here. Here's a concrete proposal.  It requires
> adding a stanza to trac.ini which I'll append to this message.
> 
> A user opens a trac bug/change ticket, say 1234.
> 
> Management assigns the ticket to Minion (or Minion always
> gets this class of ticket). This is naturally tracked by trac.
> 
> Minion fixes it on an svn branch.  I propose that
> we add a directory "tickets" to the top-level
> triumvirate of trunk/branches/tags.  So the svn command is
> something like
>   svn cp file:///scr0/tracrepo/trunk file:///scr0/tracrepo/tickets/
> 1234
> After the fix is made, Minion checks it in; this will be some
> numerical revision number, say 666.  The ci message references
> the ticket number as #1234.
> 
> This proposed fix now exists in svn with a
> well-defined name so the fix can be added to the ticket,
> with a reference to [666], and the ticket can be assigned to
> Reviewer with Review state "needsReview".
> 
> Reviewer can co the fixed version (or more probably
> svn switch to it, which requires almost no network
> traffic and probably only a trivial make).  Trac has nice
> tools for visualising the changes accessible via the [666]
> in the trac ticket. Using the svn revision number rather
> than the trac ticket is a little unfortunate, but the link
> includes the full tickets/1234 repository link.
> 
> After prodding at the code and running the tests, if
> Reviewer's unhappy, she sends the ticket back to Minion for
> more work (on the same branch).  If happy, Reviewer signs off
> in the ticket, changing needsReview to reviewed and reassigning
> it to Minion.
> 
> Now that it's reviewed and accepted, Minion merges the
> branch back to the main line and closes the ticket.
> 
> This seems pretty nice to me.  The whole process is managed and there's
> an audit trail.  You can see this working at
> http://jeeves.astro.princeton.edu/trac/ticket/3
> (ticket 2 shows me working out how to do this, so ignore it.  Also
> we don't have authentication quite working, which is, I think, why
> it keeps thinking that I'm anonymous).
> 
> Also, I don't have things set up to do email; we'd like to get Reviewer
> automatically notified that they've got a new job to do.  Maybe we
> need a script to assign reviewers that also sends them email with
> a link to their ticket?  I suspect that this script can be done on
> the web, linked into trac, and made to add the reviewer to the CC
> list on the ticket -- but I haven't tried this.
> 
> Jeff's worried about 1-line fixes.  Well, I'd say that if the author
> finds a bug in his code, then he simply fixes it in svn.  If someone
> else finds the bug and feels confident enough to fix it, then she
> fixes it in svn.  Otherwise, it's worth filing a bug report.  I'd
> say that anytime a bug report is created, it's worth following
> the above route.  If we found that this was too heavy weight, then
> we could relax the rules, by e.g. allowing the fixer to send a
> proposed patch by email.  My bet is that we'll find that using
> svn to handle our patches is easier.  We can clearly also shorten
> the number of trac transactions in trivial cases.
> 
> R
> 
> 
> Add this to trac.ini:
> 
> [ticket-custom]
> reviewStatus = radio
> reviewStatus.label = Review
> reviewStatus.options = notReady|needsReview|reviewed
> reviewStatus.value = 0
> 
> 
> _______________________________________________
> LSST-data mailing list
> [email protected]
> http://www.lsstmail.org/mailman/listinfo/lsst-data
> 

_______________________________________________
LSST-data mailing list
[email protected]
http://www.lsstmail.org/mailman/listinfo/lsst-data

Reply via email to