You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Julian Foad <ju...@btopenworld.com> on 2008/10/23 10:23:46 UTC
Re: svn commit: r33855 - in
branches/tree-conflicts-notify/subversion: include libsvn_client libsvn_wc
svn tests/cmdline tests/cmdline/svntest
On Thu, 2008-10-23 at 01:59 -0700, Greg Stein wrote:
> On Wed, Oct 22, 2008 at 6:04 PM, <ne...@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@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion: include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by Greg Stein <gs...@gmail.com>.
Oh no no no... I wasn't suggesting that you change anything other than
to stop saying "don't bother reviewing this". But that was my own lack
of awareness of the intended size of your patch (as Julian rightfully
and insightfully pointed out).
So: keep going! :-)
Cheers,
-g
On Thu, Oct 23, 2008 at 3:59 AM, Neels J. Hofmeyr <ne...@elego.de> wrote:
> 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, <ne...@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@subversion.tigris.org
>> For additional commands, e-mail: dev-help@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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion:
include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by "Neels J. Hofmeyr" <ne...@elego.de>.
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, <ne...@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@subversion.tigris.org
> For additional commands, e-mail: dev-help@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
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion:
include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Greg Stein wrote on Thu, 23 Oct 2008 at 04:09 -0700:
> > Is it the question of being easily able to know what to review that's
> > most important here?
>
> Yes. And to that point, let me formally suggest a /patches/ directory,
> as a sibling to /branches/. We can put developer-work in there, and we
> can also drop third-party patches in there, too, pending
> review/tweaking/application (rather than attach them to the issue
> tracker, for example).
>
> Thoughts?
>
Throwing fell-through-the-cracks third-party patches in the repository is
reasonable, and (with my patch manager hat on) not much effort. Throwing
*all* third-party patches to temporary branches in the repository before
applying them seems to me to introduce unnecessary overhead (but I'm sure
you weren't suggesting that :) ).
Daniel
> Cheers,
> -g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion:
include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by "Neels J. Hofmeyr" <ne...@elego.de>.
Greg Stein wrote:
> On Thu, Oct 23, 2008 at 4:19 AM, Neels J Hofmeyr <ne...@elego.de> wrote:
>> ...
>> Is it safe to move the branches/tree-conflicts-notify to patches/ once it's
>> agreed or am I likely to run into problems as the one I just reposted? :P
>> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=144469
>
> It *should* be safe. Whether svn works properly or not is a different
> question, and a bug for us to fix (imo). Renames like that should Just
> Work.
>
> That said: I don't know enough about what happened with your prior
> move, or why this might not work (if it is broken at all).
Hm, interesting -- I can't reproduce it when running a standalone recipe on
trunk, but when doing the following, using svn compiled from trunk:
cd ~/trunk/subversion/tests/cmdline
svn log -r 31016
, the log is empty, although it should show the original creation log of the
file subversion/tests/cmdline/tree_conflict_tests.txt (moved from
notes/tree-conflicts/testing.txt). When doing the same a couple dirs up:
cd ~/trunk
svn log -r 31016
, the log message shows correctly. This only happens with log -r <rev>, not
with log <filename>.
Might there be something wrong with the information stored in our svn repos
due to use of an older client at the time of the move? Or something?
I'm pretty sure that the results of above example must be wrong. It persists
on trunk but can't be reproduced separately, apparently. It can't be related
to using the http:// protocol, can it? (Attaching test recipe)
~Neels
--
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
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion: include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by Greg Stein <gs...@gmail.com>.
On Thu, Oct 23, 2008 at 4:19 AM, Neels J Hofmeyr <ne...@elego.de> wrote:
>...
> Is it safe to move the branches/tree-conflicts-notify to patches/ once it's
> agreed or am I likely to run into problems as the one I just reposted? :P
> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=144469
It *should* be safe. Whether svn works properly or not is a different
question, and a bug for us to fix (imo). Renames like that should Just
Work.
That said: I don't know enough about what happened with your prior
move, or why this might not work (if it is broken at all).
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion:
include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by Neels J Hofmeyr <ne...@elego.de>.
Greg Stein wrote:
> All very good points, Julian! Very much agreed. Thanks.
>
> I think the heart of the problem lies in "what *style* is this
> branch?" as you note below:
>
[...]
>
> Yes. And to that point, let me formally suggest a /patches/ directory,
> as a sibling to /branches/. We can put developer-work in there, and we
> can also drop third-party patches in there, too, pending
> review/tweaking/application (rather than attach them to the issue
> tracker, for example).
+1, good idea.
Is it safe to move the branches/tree-conflicts-notify to patches/ once it's
agreed or am I likely to run into problems as the one I just reposted? :P
http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=144469
~Neels
>
> Thoughts?
>
> Cheers,
> -g
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@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
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion:
include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by "Neels J. Hofmeyr" <ne...@elego.de>.
Stefan Sperling wrote:
> On Thu, Oct 23, 2008 at 07:36:50AM -0500, Hyrum K. Wright wrote:
>> Greg Stein wrote:
>>> Yes. And to that point, let me formally suggest a /patches/ directory,
>>> as a sibling to /branches/. We can put developer-work in there, and we
>>> can also drop third-party patches in there, too, pending
>>> review/tweaking/application (rather than attach them to the issue
>>> tracker, for example).
>> I think this would be a good way to help tidy up /branches/ we've got at least a
>> few trees in there which were added as patches, and then never touched again.
>> Having a way to differentiate them from active, long-lived branches would be useful.
>
> So what are active, long-lived branches and what are patches?
>
> To me this looks like we'd end up having just stable release branches
> in /branches/, and everything else that's not in trunk (yet) in /patches/?
>
> I've added a branch recently to work on issue #2382. I created the branch
> for the same reasons Julian mentioned -- I started working on a patch,
> found that I needed to checkpoint my work because I need to experiment
> a bit to get it right. So I created a branch. The aim for the end result
> is to get a reviewable patch that sits on the branch, ready to be merged
> into trunk. Since the /branches/ directory has been used like this for quite
What's new :P
;)
> some time, and because it's not clear to me where to draw the line between
> "active & long-lived" and "patch" (if not between release branches and
> everything else), I don't see why we need another directory.
>
> But I don't care too much -- as long as I can have some place in the repo
> to checkpoint my work without having to step on anyone's toes, I'm happy.
I'd like to think of it more like "is this a branch that everyone should
review closely, or is it rather a smaller issue, or even just a trial, that
I may want to show to those interested but don't need to bother everyone with."
Do you think it's dangerous to have that around? As in "oh, he just merged
his patch to trunk, I'm not gonna review that now."
Otherwise, I'd say, we can leave everyone to draw their personal line
themselves and just happily use branches/ or patches/ whenever it suits them.
Wow, are we bikeshedding already?
~Neels
Re: svn commit: r33855 - in
branches/tree-conflicts-notify/subversion: include libsvn_client
libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Oct 23, 2008 at 07:36:50AM -0500, Hyrum K. Wright wrote:
> Greg Stein wrote:
> > Yes. And to that point, let me formally suggest a /patches/ directory,
> > as a sibling to /branches/. We can put developer-work in there, and we
> > can also drop third-party patches in there, too, pending
> > review/tweaking/application (rather than attach them to the issue
> > tracker, for example).
>
> I think this would be a good way to help tidy up /branches/ we've got at least a
> few trees in there which were added as patches, and then never touched again.
> Having a way to differentiate them from active, long-lived branches would be useful.
So what are active, long-lived branches and what are patches?
To me this looks like we'd end up having just stable release branches
in /branches/, and everything else that's not in trunk (yet) in /patches/?
I've added a branch recently to work on issue #2382. I created the branch
for the same reasons Julian mentioned -- I started working on a patch,
found that I needed to checkpoint my work because I need to experiment
a bit to get it right. So I created a branch. The aim for the end result
is to get a reviewable patch that sits on the branch, ready to be merged
into trunk. Since the /branches/ directory has been used like this for quite
some time, and because it's not clear to me where to draw the line between
"active & long-lived" and "patch" (if not between release branches and
everything else), I don't see why we need another directory.
But I don't care too much -- as long as I can have some place in the repo
to checkpoint my work without having to step on anyone's toes, I'm happy.
Stefan
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion:
include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Greg Stein wrote:
> Yes. And to that point, let me formally suggest a /patches/ directory,
> as a sibling to /branches/. We can put developer-work in there, and we
> can also drop third-party patches in there, too, pending
> review/tweaking/application (rather than attach them to the issue
> tracker, for example).
I think this would be a good way to help tidy up /branches/ we've got at least a
few trees in there which were added as patches, and then never touched again.
Having a way to differentiate them from active, long-lived branches would be useful.
-Hyrum
Re: svn commit: r33855 - in branches/tree-conflicts-notify/subversion: include libsvn_client libsvn_wc svn tests/cmdline tests/cmdline/svntest
Posted by Greg Stein <gs...@gmail.com>.
All very good points, Julian! Very much agreed. Thanks.
I think the heart of the problem lies in "what *style* is this
branch?" as you note below:
2008/10/23 Julian Foad <ju...@btopenworld.com>:
>...
> 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.
Bingo. I was unaware that Neels' branch was intended to be a "public
development of a patch". In that sense, then I'm very glad it is
there, for all the reasons that you point out. BUT. I didn't know that
it would be subject to (good) review when it finally got merged, on
the basis of it being reviewable-sized.
I think your suggestion would be a fine idea. Something like
/patches/tree-conflict-notify/ would work well.
>...
> 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.
No, my comment wasn't about trusting ourselves, but a lack of
awareness of the *intended* size of Neels' work.
But your *is* true. Some of the branches get a bit big, or too
long-lived, or whatever.
> Is it the question of being easily able to know what to review that's
> most important here?
Yes. And to that point, let me formally suggest a /patches/ directory,
as a sibling to /branches/. We can put developer-work in there, and we
can also drop third-party patches in there, too, pending
review/tweaking/application (rather than attach them to the issue
tracker, for example).
Thoughts?
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org