Format for JDK 6/7 changeset comments?

Kelly O'Hair Kelly.Ohair at Sun.COM
Wed Nov 7 17:04:58 UTC 2007



Andrew Haley wrote:
> Kelly O'Hair writes:
> 
>  > Mercurial changesets should not be created until the change has
>  > been reviewed and is ready to be integrated into a public area.
> 
>  > Changes that are untested, unreviewed, or experimental should stay
>  > as patches, at least that's my opinion after using Mercurial for
>  > some time.
> 
>  > If we allow completely unreviewed and untested changesets into the
>  > repositories, we run the risk of severe changeset clutter. They
>  > don't have to be perfect, but we need some kind of quality control
>  > on them.
> 
> Sure, but that seems to be orthogonal to what I wrote.
> 
> I am trying to make sure that all the information about every change
> is in a searchable form that doesn't require special software.  I'm
> trying to make sure that if (God forbid) a nuke fell on Sun central,
> everyone else would just carry on with their work, with all of the
> information they need easily to have.

I'm not directly involved in the bug tracking system plans, but I'd be
very surprised if the system we pick isn't searchable, it certainly
will be open. I know that until we have a completely open bug tracking
system, this could be an issue, but I really hate to design around
the current status when we know it will be changing.

> 
> I'll explain what we do at GNU, and why I think it's a good thing.
> 
> The comment we write for a change is written and submitted for review
> along with the patch.  When the patch is approved, this pre-written
> comment is used for the version control commit.  So, when you look at
> a patch submission you can immediately tie it with the commit.

The problem is that with code, you have reviewers looking at it,
compilers compiling it, various tools looking for problems with it,
tests running it, etc. But comments, as valuable as they are, are just
words, and if they are wrong, the odds are very high that it won't be detected.
The longer the comment, the better the odds of it having mistakes.
And once baked in, those incorrect words live on forever.

I'm not against comments by any means, but it just seems like putting them
somewhere closer to the actual bug report, in a system that allows you
to correct and expand upon later makes the most sense to me.

And there is also the "one source of the truth" concept too. I'd like to
know that the changeset is the one source of the code changes, and the
bug report is the one source of what the bug is.

> 
>  > People need to try the mq extension (myself included), which may
>  > provide some answers here.
>  > 
>  > And the notifications of changesets that have been integrated can
>  > include the diffs, but when you can easily and quickly browse the
>  > changeset, I'm not so sure you need to send all the diffs.
> 
> Agreed, but that's a matter of making sure that the changeset, once
> checked in, really was exactly the same as what was reviewed.
> Redundancy here is a good thing.

Redundancy is good when it helps find errors, so in terms of getting
a change more exposure during a review or the initial push, sure this
might allow for someone to catch a problem.

I guess I was thinking more along the lines of archeology, I tend to
not trust email diffs.

-kto

> 
>  > But that's a changeset notification issue, not a changeset comment
>  > format issue.
> 
> True enough.
> 
> Andrew.
> 
> 
>  > Andrew Haley wrote:
>  > > Kelly O'Hair writes:
>  > >  > I agree.
>  > >  > 
>  > >  > The more you put in the changeset comment, the higher the odds that
>  > >  > there will be mistakes in those comments, mistakes that can NEVER
>  > >  > be corrected.
>  > >  > I favor keeping it short and sweet, and use the bug database for
>  > >  > all other information.  A place that can be corrected and added to
>  > >  > over time.
>  > >  > 
>  > >  > Of course the bug database needs to refer to the changeset, which
>  > >  > IS the true source of the change. Any webrevs and diffs in the bug
>  > >  > database should probably be removed once a changeset is public, or
>  > >  > perhaps multiple changesets depending on how many it takes to
>  > >  > really fix a bug. You don't want incorrect diffs or webrevs
>  > >  > floating around when the true change is in the changeset.
>  > > 
>  > > Hmm.  Does this mean that the checkin comments are not written until
>  > > after the reviewer has done their work?  That seems wrong.  In gcc we
>  > > write the change log entry when we submit a change.
>  > > 
>  > > Also, IMO the supplemental information about the bug should be
>  > > automatically copied into a mailing list as part of the same thread to
>  > > which the change itself was copied.  That way, you only need mailing
>  > > list searching software to find everything.  It would be better not to
>  > > depend on the bug database in order to find out why something has
>  > > changed, or indeed what has changed.
>  > > 
>  > > Andrew.
>  > > 
> 



More information about the discuss mailing list