You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "C. Michael Pilato" <cm...@collab.net> on 2007/10/12 02:42:52 UTC

Re: svn checkpoint: r27139 - checkpoints

mailer.py on svn.collab.net has been tweaked to, for commits which touch
this checkpoints directory:

   Use 'svn checkpoint' instead of 'svn commit' in the subject line
   Suppress all diffs
   Suppress all ViewVC URLs

(This mail was a test re-generation of the commit mail for r27139.)


glasser@tigris.org wrote:
> Author: glasser
> Date: Thu Oct 11 19:06:44 2007
> New Revision: 27139
> 
> Log:
> While Subversion's branches are lightweight, there's some social
> overhead involved in making a new branch in /branches in our
> repository.  It's not something you want to do for quickly
> checkpointing half-written code: after all, every commit sends out
> mail, and you don't want to waste other developer's time reviewing
> patches you know are unfinished.
> 
> On the other hand, if the commit messages were turned off (or
> redirected to a different mailing list that only the most paranoid
> developers were on), the repository would be a great place for
> committers to checkpoint big patches.  So let's just make a separate
> directory for "below the radar" branches: /checkpoints.
> 
> (This idea seemed good to some of us on IRC.)
> 
> Note that you shouldn't actually use this until somebody changes the
> mailer hooks.
> 
> 
> Added:
>    checkpoints/
>    checkpoints/README
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: svn-help@subversion.tigris.org
> 


-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn checkpoint: r27139 - checkpoints

Posted by "C. Michael Pilato" <cm...@collab.net>.
Karl Fogel wrote:
> "C. Michael Pilato" <cm...@collab.net> writes:
>> Exactly.  You can't see the diffs when Karl squirrels away patches in his
>> local disk, either, and that's fine.  What matters is the change dropped
>> into a live branch -- that's the diff to review.  If Karl wants early
>> feedback on his work in progress, then he needs to use a regular feature
>> branch and work within the established policies for commits to our tree.
> 
> Things are not quite so easily defined, I think.
> 
> For example, attaching successive patches to an issue has never part
> of our process, but I've been doing it (for off-site backups, and to
> provide review opportunities to anyone intensely interested in that
> issue's progress).
> 
> 90% of the time, I'm the only one watching those patches.  But every
> now and then, I do get reviews, and they're helpful (it happened most
> recently in issue #2959, with both dlr and vgeorgescu).  Did I "want"
> those reviews?  Well, I didn't expect them, but I was glad to get
> them.

Great benefit to preserve, absolutely.  But the nature of our issue tracker
and patch attachments to it are such that folks don't choose to offer their
unsolicited review because the patch flashed across their eyes while reading
email.  'svn diff -c SOME-REV URL-OF-CHECKPOINTS' is no harder for the
competent than launching a web browser and viewing a patch there.

> So there can be fine gradations of desire/need for review.  I think
> Justin's instincts are right, that if there's going to be a review
> opportunity at all, we should at least try to make it easy.

And that's the right attitude to have.  FWIW, I'd be perfectly happy with
reverting the special mailer changes on svn.collab.net and just having a
sort of known policy that in /checkpoints, our commit policies aren't as strict.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn checkpoint: r27139 - checkpoints

Posted by Karl Fogel <kf...@red-bean.com>.
"C. Michael Pilato" <cm...@collab.net> writes:
> Exactly.  You can't see the diffs when Karl squirrels away patches in his
> local disk, either, and that's fine.  What matters is the change dropped
> into a live branch -- that's the diff to review.  If Karl wants early
> feedback on his work in progress, then he needs to use a regular feature
> branch and work within the established policies for commits to our tree.

Things are not quite so easily defined, I think.

For example, attaching successive patches to an issue has never part
of our process, but I've been doing it (for off-site backups, and to
provide review opportunities to anyone intensely interested in that
issue's progress).

90% of the time, I'm the only one watching those patches.  But every
now and then, I do get reviews, and they're helpful (it happened most
recently in issue #2959, with both dlr and vgeorgescu).  Did I "want"
those reviews?  Well, I didn't expect them, but I was glad to get
them.

So there can be fine gradations of desire/need for review.  I think
Justin's instincts are right, that if there's going to be a review
opportunity at all, we should at least try to make it easy.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by "Brian W. Fitzpatrick" <fi...@red-bean.com>.
On 10/18/07, Karl Fogel <kf...@red-bean.com> wrote:
>
> Somebody should document this somewhere!
>
> I just did, in hacking.html.


Karl, I've always loved you.

-Fitz

Re: svn checkpoint: r27139 - checkpoints

Posted by David Glasser <gl...@davidglasser.net>.
On 10/18/07, Karl Fogel <kf...@red-bean.com> wrote:
> "Brian W. Fitzpatrick" <fi...@red-bean.com> writes:
> > And if Karl *doesn't* want feedback on a branch, what's wrong with
> > prepending this to the log message:
> >
> >    *** This is a checkpoint commit--please don't waste your time
> > reviewing it ***
> >
> > I cannot even begin to comprehend this whole business about checking
> > diffs into a directory that our commit mailer ignores.  We have a
> > version control system, we have branches.  Ben and I have given
> > numerous talks on the importance of committing early and often.  We
> > seem to be diving deep into a minor social problem with a half-baked
> > technological solution that just plain doesn't work with existing
> > version control tools (ever look at a diff of unified diffs?  Enough
> > to melt your brain...).
> >
> > So, to summarize:  Please please PLEASE just do your work on a feature
> > branch, prefix your commit messages with a warning not to review, and
> > then encourage folks to review when you merge back to trunk.  Nobody
> > requires that a feature branch compiles or contains code ready for
> > review at all times.
> >
> > Please?
>
> Sure.  Loosening up our (hitherto implicit) policies about creating
> feature or experimental branches would be essentially equivalent to
> using the "/checkpoint" or "/anarchy" directory.  But I don't see any
> reason to prefer the latter solution -- the former is just as good.
>
> When a specific commit should not be reviewed, just say so.  If later
> commits on that branch *should* be reviewed, it might be nice for the
> author to supply (in the log message) the appropriate 'svn diff'
> command, since the diff will likely be between two non-adjacent revs
> on that branch.  I.e., review doesn't have to wait until the changes
> hit trunk.
>
> Somebody should document this somewhere!
>
> I just did, in hacking.html.
>
> r27267,
> -Karl

Sounds good to me.  I think we should be explicit in HACKING as to
whether or not partial committers should feel free to do this as well.
 (I say sure.)

--dave

-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by Karl Fogel <kf...@red-bean.com>.
"Brian W. Fitzpatrick" <fi...@red-bean.com> writes:
> And if Karl *doesn't* want feedback on a branch, what's wrong with
> prepending this to the log message:
>
>    *** This is a checkpoint commit--please don't waste your time
> reviewing it ***
>
> I cannot even begin to comprehend this whole business about checking
> diffs into a directory that our commit mailer ignores.  We have a
> version control system, we have branches.  Ben and I have given
> numerous talks on the importance of committing early and often.  We
> seem to be diving deep into a minor social problem with a half-baked
> technological solution that just plain doesn't work with existing
> version control tools (ever look at a diff of unified diffs?  Enough
> to melt your brain...).
>
> So, to summarize:  Please please PLEASE just do your work on a feature
> branch, prefix your commit messages with a warning not to review, and
> then encourage folks to review when you merge back to trunk.  Nobody
> requires that a feature branch compiles or contains code ready for
> review at all times.
>
> Please?

Sure.  Loosening up our (hitherto implicit) policies about creating
feature or experimental branches would be essentially equivalent to
using the "/checkpoint" or "/anarchy" directory.  But I don't see any
reason to prefer the latter solution -- the former is just as good.

When a specific commit should not be reviewed, just say so.  If later
commits on that branch *should* be reviewed, it might be nice for the
author to supply (in the log message) the appropriate 'svn diff'
command, since the diff will likely be between two non-adjacent revs
on that branch.  I.e., review doesn't have to wait until the changes
hit trunk.

Somebody should document this somewhere!

I just did, in hacking.html.

r27267,
-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by "Brian W. Fitzpatrick" <fi...@red-bean.com>.
On 10/13/07, C. Michael Pilato <cm...@collab.net> wrote:
> Exactly.  You can't see the diffs when Karl squirrels away patches in his
> local disk, either, and that's fine.  What matters is the change dropped
> into a live branch -- that's the diff to review.  If Karl wants early
> feedback on his work in progress, then he needs to use a regular feature
> branch and work within the established policies for commits to our tree.

And if Karl *doesn't* want feedback on a branch, what's wrong with
prepending this to the log message:

   *** This is a checkpoint commit--please don't waste your time
reviewing it ***

I cannot even begin to comprehend this whole business about checking
diffs into a directory that our commit mailer ignores.  We have a
version control system, we have branches.  Ben and I have given
numerous talks on the importance of committing early and often.  We
seem to be diving deep into a minor social problem with a half-baked
technological solution that just plain doesn't work with existing
version control tools (ever look at a diff of unified diffs?  Enough
to melt your brain...).

So, to summarize:  Please please PLEASE just do your work on a feature
branch, prefix your commit messages with a warning not to review, and
then encourage folks to review when you merge back to trunk.  Nobody
requires that a feature branch compiles or contains code ready for
review at all times.

Please?

-Fitz

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by "C. Michael Pilato" <cm...@collab.net>.
Jay Levitt wrote:
> On 10/13/2007 12:12 PM, Justin Erenkrantz wrote:
>> On Oct 13, 2007 12:55 AM, Karl Fogel <kf...@red-bean.com> wrote:
>>> How about: "anything you would have formerly attached to an issue as a
>>> patch", for starters?  Now we can just say a revision number instead.
>>
>> But, then, it's impossible to review it via email because we don't
>> have diffs.
>>
>> All I'm suggesting is that we turn back on diffs and socially tell
>> folks, "don't be a turd" about giving nit-pick comments to anything in
>> /checkpoints.  Yet, if the intent of /checkpoints is that
>> occassionally folks will be asked to look in it, then that implies we
>> need easy ways to review what is there.  -- justin
> 
> If I'm understanding, /checkpoints is essentially a centralized,
> versioned working copy.  So you really don't want diffs internal to a
> checkpoint.
> 
> If the diffs don't show up again when you merge a checkpoint into trunk,
> then that's the bug that needs fixing.

Exactly.  You can't see the diffs when Karl squirrels away patches in his
local disk, either, and that's fine.  What matters is the change dropped
into a live branch -- that's the diff to review.  If Karl wants early
feedback on his work in progress, then he needs to use a regular feature
branch and work within the established policies for commits to our tree.

-- 
C. Michael Pilato <cm...@collab.net>
CollabNet   <>   www.collab.net   <>   Distributed Development On Demand


Re: svn checkpoint: r27139 - checkpoints

Posted by Jay Levitt <li...@shopwatch.org>.
On 10/13/2007 12:12 PM, Justin Erenkrantz wrote:
> On Oct 13, 2007 12:55 AM, Karl Fogel <kf...@red-bean.com> wrote:
>> How about: "anything you would have formerly attached to an issue as a
>> patch", for starters?  Now we can just say a revision number instead.
> 
> But, then, it's impossible to review it via email because we don't have diffs.
> 
> All I'm suggesting is that we turn back on diffs and socially tell
> folks, "don't be a turd" about giving nit-pick comments to anything in
> /checkpoints.  Yet, if the intent of /checkpoints is that
> occassionally folks will be asked to look in it, then that implies we
> need easy ways to review what is there.  -- justin

If I'm understanding, /checkpoints is essentially a centralized, 
versioned working copy.  So you really don't want diffs internal to a 
checkpoint.

If the diffs don't show up again when you merge a checkpoint into trunk, 
then that's the bug that needs fixing.

Jay Levitt

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by Karl Fogel <kf...@red-bean.com>.
Jay Levitt <li...@shopwatch.org> writes:
> I guess the thing I'm not getting is:
>
> When you merge a checkpoint back into trunk, does mailer.py send the
> diff of the entire merge?

Personally, I'm not going to merge my checkpoints that way.  I'm going
to commit them from my working copy, just as if I'd written them from
scratch (which, hey, I did :-) ).  

I'm just going to use the checkpoints area ("/checkpoints" or
"/anarchy/checkpoints" or whatever it is) as a way to save
work-in-progress visibly, but not as a merge source.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by Jay Levitt <li...@shopwatch.org>.
On 10/14/2007 9:55 AM, Karl Fogel wrote:
> "Justin Erenkrantz" <ju...@erenkrantz.com> writes:
>> On Oct 13, 2007 12:55 AM, Karl Fogel <kf...@red-bean.com> wrote:
>>> How about: "anything you would have formerly attached to an issue as a
>>> patch", for starters?  Now we can just say a revision number instead.
>> But, then, it's impossible to review it via email because we don't have diffs.
> 
> No objection here to turning diffs back on for /checkpoints.

I guess the thing I'm not getting is:

When you merge a checkpoint back into trunk, does mailer.py send the 
diff of the entire merge?

If it does, then great.. people can do whatever they want in 
/checkpoints or /anarchy or whatever it's called.  If they happen to 
point someone to a /checkpoints release, it's just like uploading a 
patch.  If they don't, then it's their private whatever.

And when they merge back in, we see the whole diff.

If it doesn't... could it?  Because that seems useful for all sorts of 
reasons.

Jay

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by Karl Fogel <kf...@red-bean.com>.
"Justin Erenkrantz" <ju...@erenkrantz.com> writes:
> On Oct 13, 2007 12:55 AM, Karl Fogel <kf...@red-bean.com> wrote:
>> How about: "anything you would have formerly attached to an issue as a
>> patch", for starters?  Now we can just say a revision number instead.
>
> But, then, it's impossible to review it via email because we don't have diffs.

No objection here to turning diffs back on for /checkpoints.

> All I'm suggesting is that we turn back on diffs and socially tell
> folks, "don't be a turd" about giving nit-pick comments to anything in
> /checkpoints.  Yet, if the intent of /checkpoints is that
> occassionally folks will be asked to look in it, then that implies we
> need easy ways to review what is there.  -- justin

Agreed.  (Sorry, I didn't grok the core of your complaint before.)

Now, one usage I forsee is

   $ svn cp -m "checkpointing current code" . http://.../checkpoints/

In that case, will it show just the diff against the files' baselines
(what I hope), or will it show the entire tree (what I fear)?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Oct 13, 2007 12:55 AM, Karl Fogel <kf...@red-bean.com> wrote:
> How about: "anything you would have formerly attached to an issue as a
> patch", for starters?  Now we can just say a revision number instead.

But, then, it's impossible to review it via email because we don't have diffs.

All I'm suggesting is that we turn back on diffs and socially tell
folks, "don't be a turd" about giving nit-pick comments to anything in
/checkpoints.  Yet, if the intent of /checkpoints is that
occassionally folks will be asked to look in it, then that implies we
need easy ways to review what is there.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by Karl Fogel <kf...@red-bean.com>.
Blair Zajac <bl...@orcaware.com> writes:
> What's the standard for checkpoints?

How about: "anything you would have formerly attached to an issue as a
patch", for starters?  Now we can just say a revision number instead.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn checkpoint: r27139 - checkpoints

Posted by Blair Zajac <bl...@orcaware.com>.
What's the standard for checkpoints?  Do we explicitly state that the checkpoint 
was merged into the trunk with a simple log message indicating the source of the 
merge or do we log the merge into trunk as an original commit?

Given that checkpoints don't have to have properly formed log messages, I think 
the later would be better.  You probably don't want people to have to review the 
individual commits in the checkpoints since they may be lower quality.

BTW, I've checkpointed the relative externals code.  I want to give it one more 
pass and spit-and-shine on it so it's for trunk.

Blair

C. Michael Pilato wrote:
> mailer.py on svn.collab.net has been tweaked to, for commits which touch
> this checkpoints directory:
> 
>    Use 'svn checkpoint' instead of 'svn commit' in the subject line
>    Suppress all diffs
>    Suppress all ViewVC URLs
> 
> (This mail was a test re-generation of the commit mail for r27139.)
> 
> 
> glasser@tigris.org wrote:
>> Author: glasser
>> Date: Thu Oct 11 19:06:44 2007
>> New Revision: 27139
>>
>> Log:
>> While Subversion's branches are lightweight, there's some social
>> overhead involved in making a new branch in /branches in our
>> repository.  It's not something you want to do for quickly
>> checkpointing half-written code: after all, every commit sends out
>> mail, and you don't want to waste other developer's time reviewing
>> patches you know are unfinished.
>>
>> On the other hand, if the commit messages were turned off (or
>> redirected to a different mailing list that only the most paranoid
>> developers were on), the repository would be a great place for
>> committers to checkpoint big patches.  So let's just make a separate
>> directory for "below the radar" branches: /checkpoints.
>>
>> (This idea seemed good to some of us on IRC.)
>>
>> Note that you shouldn't actually use this until somebody changes the
>> mailer hooks.
>>
>>
>> Added:
>>    checkpoints/
>>    checkpoints/README
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: svn-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: svn-help@subversion.tigris.org
>>
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org