What Julian said, and (until told otherwise), I assumed I would need to
write a new, complete log message for all the changes upon `merge
--reintegrate'. Wouldn't that be a good solution for the reviewing problem?
Then again, when following that scheme, `blame' would show a revision number
that doesn't lead to the final (merging) log message for that line, but only
to the (committing) one that was deemed intermediate. Might get wormy.
So, Greg, should I continue like this or would you like me to change my
branching behaviour?
~Neels
Julian Foad wrote:
> On Thu, 2008-10-23 at 01:59 -0700, Greg Stein wrote:
>> On Wed, Oct 22, 2008 at 6:04 PM, <neels_at_tigris.org> wrote:
>>> Author: neels
>>> Date: Wed Oct 22 18:04:30 2008
>>> New Revision: 33855
>>>
>>> Log:
>>> On branch: tree-conflicts-notify
>>>
>>> *** This is just a start. Don't waste too much time reviewing it! ***
>>> *** Some parts of this may be misguided and go away again. ***
>> Neels,
>>
>> EVERY commit should be fully-reviewed, and fully reviewable.
>
> Not so, not on a "light weight" branch. We wrote in our "hacking.html":
>
> [[[
> Use lightweight branches
> ------------------------
> If you're working on a feature or bugfix in stages involving multiple
> commits, and some of the intermediate stages aren't stable enough to go
> on trunk, then create a temporary branch in /branches. There's no need
> to ask — just do it. It's fine to try out experimental ideas in a
> temporary branch, too. And all the preceding applies to partial as well
> as full committers.
>
> If you're just using the branch to "checkpoint" your code, and don't
> feel it's ready for review, please put some sort of notice at the top of
> the log message, such as:
>
> *** checkpoint commit -- please don't waste your time reviewing it ***
>
> [...]
> ]]]
>
>
> So what Neels is doing is fine.
>
>
> Now, you may be right in terms of defining a better policy. That's
> certainly one good policy for working on a branch, especially on a
> feature development that is shared and/or long-lived. However, there are
> other uses for a branch.
>
> I think Neels intended this branch to be for developing a small change,
> no bigger than a patch that would otherwise be sent to the mailing list
> (and it still can be). It's like a developer-private branch, an
> alternative to saving "svn diff" patches every hour or local commits
> into SVK. The fact that it is publically visible is a bonus.
>
> Some of us "think" by writing a chunk of code, looking at how it turns
> out, trying to work with it, and then re-writing it as necessary. If we
> forbid developers from working in that way, they are likely to go back
> to saving local patch files or check in to SVK, with less visibility and
> less incentive to think about the reviewability and wholeness of each
> such stage. It would probably not encourage them to write more complete
> intermediate steps when the aim is itself just a small change.
>
> Does that matter? Maybe that would be better because we would all know
> exactly which commits are supposed to be reviewed (all of them) which
> would avoid the tendency for us to start ignoring all commits on
> branches. If that's the main problem (and it certainly could become a
> problem - we already review branches far less than we should), then we
> should at least consider addressing it in a different way, such as using
> a "/branches/scratch/" name space.
>
>> Why? The answer is simple. If a review is not performed, then *when*
>> should be it be done? We all agree reviews should be done, so then
>> when, if not when it first gets committed?
>>
>> "Branch merge" is not the right answer. At that point, the overall
>> change will be too large to properly review.
>
> That's the crux here: that's not necessarily true. Yes, it's easy to get
> into that situation, and it often happens, but must we assume it will
> happen?
>
> I guess you're saying that we can't trust most of ourselves most of the
> time to stick to a reviewable-sized change, and then make sure to get it
> reviewed. And historically that's been so. So maybe overall it would be
> better not to work in this way.
>
> Is it the question of being easily able to know what to review that's
> most important here?
>
> - Julian
>
>
>> This is the second or third time, I've seen your log message basically
>> say "don't bother to review this". I think that is an incorrect
>> procedure/direction, so I wanted to raise this point about reviewable
>> code. Also note this means you should consider your commits to a
>> branch to be just as focused, complete, and working as any commit you
>> would make to /trunk.
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org
>
--
Neels Hofmeyr -- elego Software Solutions GmbH
Gustav-Meyer-Allee 25 / Gebäude 12, 13355 Berlin, Germany
phone: +49 30 23458696 mobile: +49 177 2345869 fax: +49 30 23458695
http://www.elegosoft.com | Geschäftsführer: Olaf Wagner | Sitz: Berlin
Handelsreg: Amtsgericht Charlottenburg HRB 77719 | USt-IdNr: DE163214194
Received on 2008-10-23 13:00:04 CEST