You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Blair Zajac <bl...@orcaware.com> on 2002/08/02 22:52:58 UTC
Holding commits till review
At my work, we have a review-commit model. I'd like to try something
like this that uses Subversion's txn support.
1) Developer does work.
2) Developer commits.
3) Commit system notes that developer's commit needs review and
a) Keeps the txn in the database and quits.
b) Runs a modified commit-email.pl on the txn to send out a review
request.
c) Quits without deleting txn.
4) Somebody reviews the changes and say OKs it.
5) The manager goes to a CGI interface to the repos which lists all
the outstanding txn's and selects the txn to apply.
This is nice because then you know the exact commit being applied to
the tree. Normally in CVS you do the review and the developer may have
made some more changes to the modification that were not reviewed and
checks that in.
Is this possible? Would it work in this case?
1) Developer 1's commit gets saved for review and later commit.
2) Developer 2 has special access and commits immediately.
I'm guessing that invalidates 1's txn?
Also, do the txn's store the time that they were created in
server time so they can be properly sorted? You don't want
client time, because that may be messed up.
Best,
Blair
--
Blair Zajac <bl...@orcaware.com>
Web and OS performance plots - http://www.orcaware.com/orca/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by Casper Hornstrup <ch...@users.sourceforge.net>.
lør, 2002-08-03 kl. 19:07 skrev David Waite:
>
>
> Casper Hornstrup wrote:
>
> >lør, 2002-08-03 kl. 01:53 skrev Greg Hudson:
> >
> >
> >>Saved transactions are a possible post-1.0 Subversion feature,
> >>definitely. The hard part would be the case where auto-merging can no
> >>longer be done on the saved transaction, so that it can't be committed;
> >>do you have to recreate the transaction, or is there some way you could
> >>manually merge an existing transaction?
> >>
> >>
> >
> >So, is delayed transactions planned for Subversion? If so, I have an
> >idea for another use for them. If the state of the transaction
> >(commit/rollback) can be controlled from an external program or script,
> >then it would be possible to prevent regressions in the repository. I'm
> >developing an open source OS that has the potential to be compiled for a
> >large number of targets. As the number of targets increase, it would be
> >harder to prevent regressions or even simple compile errors on targets
> >where the patch has not been tested. If there is a large number of
> >targets, then testing a (small or large) patch on all targets will take
> >too much time. So why not integrate a regression framework with
> >subversion? Then, when a commit is attempted, the regression framework
> >checks out a copy with the attempted change, compiles it, and runs any
> >tests and if all is okay, reports back to subversion asking it to commit
> >the transaction, otherwise it tells subversion to rollback the
> >transaction.
> >
> >
> IMHO, it would be much more useful to have these commits go into
> branches rather than be transactions. Your integration environment could
> take a branch, merge it into the head revision and compile/test on all
> platforms. If it passes, it gets committed with the changes from the
> branch. If not, the developer has the list of failures and can then
> correct and resubmit.
>
> -David Waite
Would this not make it more difficult than necesarry for the developer?
I mean, the developer would then have to commit to a branch that would
be dead after the commit, right? So, after the commit, the developer
would have to switch the working copy to the HEAD branch to get other
updates. In essence, the process would be like this:
* Create a new branch and switch the working copy to it.
* Implement something on the new branch.
* Commit the patch to the new branch - and the regression framework
would then do it's thing and notify the developer by mail
(or a mailing list) of the results.
* If all okay, switch back to the HEAD branch to continue development.
* If not okay, fix and recommit.
Or did I miss something?
Casper Hornstrup
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by David Waite <ma...@akuma.org>.
Casper Hornstrup wrote:
>lør, 2002-08-03 kl. 01:53 skrev Greg Hudson:
>
>
>>Saved transactions are a possible post-1.0 Subversion feature,
>>definitely. The hard part would be the case where auto-merging can no
>>longer be done on the saved transaction, so that it can't be committed;
>>do you have to recreate the transaction, or is there some way you could
>>manually merge an existing transaction?
>>
>>
>
>So, is delayed transactions planned for Subversion? If so, I have an
>idea for another use for them. If the state of the transaction
>(commit/rollback) can be controlled from an external program or script,
>then it would be possible to prevent regressions in the repository. I'm
>developing an open source OS that has the potential to be compiled for a
>large number of targets. As the number of targets increase, it would be
>harder to prevent regressions or even simple compile errors on targets
>where the patch has not been tested. If there is a large number of
>targets, then testing a (small or large) patch on all targets will take
>too much time. So why not integrate a regression framework with
>subversion? Then, when a commit is attempted, the regression framework
>checks out a copy with the attempted change, compiles it, and runs any
>tests and if all is okay, reports back to subversion asking it to commit
>the transaction, otherwise it tells subversion to rollback the
>transaction.
>
>
IMHO, it would be much more useful to have these commits go into
branches rather than be transactions. Your integration environment could
take a branch, merge it into the head revision and compile/test on all
platforms. If it passes, it gets committed with the changes from the
branch. If not, the developer has the list of failures and can then
correct and resubmit.
-David Waite
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by Casper Hornstrup <ch...@users.sourceforge.net>.
lør, 2002-08-03 kl. 01:53 skrev Greg Hudson:
> Saved transactions are a possible post-1.0 Subversion feature,
> definitely. The hard part would be the case where auto-merging can no
> longer be done on the saved transaction, so that it can't be committed;
> do you have to recreate the transaction, or is there some way you could
> manually merge an existing transaction?
So, is delayed transactions planned for Subversion? If so, I have an
idea for another use for them. If the state of the transaction
(commit/rollback) can be controlled from an external program or script,
then it would be possible to prevent regressions in the repository. I'm
developing an open source OS that has the potential to be compiled for a
large number of targets. As the number of targets increase, it would be
harder to prevent regressions or even simple compile errors on targets
where the patch has not been tested. If there is a large number of
targets, then testing a (small or large) patch on all targets will take
too much time. So why not integrate a regression framework with
subversion? Then, when a commit is attempted, the regression framework
checks out a copy with the attempted change, compiles it, and runs any
tests and if all is okay, reports back to subversion asking it to commit
the transaction, otherwise it tells subversion to rollback the
transaction.
Casper Hornstrup
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by Greg Hudson <gh...@MIT.EDU>.
Saved transactions are a possible post-1.0 Subversion feature,
definitely. The hard part would be the case where auto-merging can no
longer be done on the saved transaction, so that it can't be committed;
do you have to recreate the transaction, or is there some way you could
manually merge an existing transaction?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by Martin Pool <mb...@sourcefrog.net>.
On 2 Aug 2002, Blair Zajac <bl...@orcaware.com> wrote:
> At my work, we have a review-commit model. I'd like to try something
> like this that uses Subversion's txn support.
You've probably already read it, but if you haven't, you might like to
look at the whitepapers about Aegis, which more-or-less enforces this
model of development. It is proudly "rules not tools", rather than
vice versa.
http://aegis.sourceforge.net/auug93.pdf
etc
--
Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by "Sergey A. Lipnevich" <se...@pisem.net>.
You might want to create an application around "svn diff | mail -s
'log'" by sending this diff to an agent which would display the changes
on a web page and provide reject/deny functionality. I imagine it would
allow an approver do download a patch, or receive it in the mail, but
from an agent and with two URLs: approve and deny. Approver likes it,
clicks approve URL, or doesn't like it, hits deny URL and has to provide
a write-up of why this wonderful piece of code was so blatantly denied,
with ready-made answers like "garbage", "doesn't work","doesn't compile
in turbo c 2.0" etc. ;-)
Blair Zajac wrote:
>Bruce Atherton wrote:
>
>
>>At 04:45 PM 8/2/02 -0700, Blair Zajac wrote:
>>David's idea of a branch per issue does solve your problem. Or you could
>>put each developer on their own branch and only allow merging a revision to
>>the trunk when an approval had been "signed" by an authorized person using
>>whatever method:email, property change, merge to trunk by someone with
>>permissions, whatever. The scary code would still be in the repository
>>(where it should be IMO) but it does no harm. And as an added bonus, you
>>can tease the person about it in the future when they finally get a clue,
>>because the evidence will still be there.
>>
>>If you are really intent on keeping the code out of the repository, another
>>solution would be to remove commit access for this developer and instead
>>have them email patches by running "svn diff | mail -s 'log message'",
>>similar to how the Subversion project handles patches from non-committers.
>>
>>
>
>We need a real diff format that could be read in to handle moves, property
>changes, etc. Like what Tom Lord has been proposing to work on.
>
>Emailing diffs is what I do now and it gets to be a pain. A nice automated
>web page showing the proposed diff and having an accept or reject button
>would sure be nice. This is similar to the Mailman interface to accepting
>posts from non-members.
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by Blair Zajac <bl...@orcaware.com>.
Bruce Atherton wrote:
>
> At 04:45 PM 8/2/02 -0700, Blair Zajac wrote:
> David's idea of a branch per issue does solve your problem. Or you could
> put each developer on their own branch and only allow merging a revision to
> the trunk when an approval had been "signed" by an authorized person using
> whatever method:email, property change, merge to trunk by someone with
> permissions, whatever. The scary code would still be in the repository
> (where it should be IMO) but it does no harm. And as an added bonus, you
> can tease the person about it in the future when they finally get a clue,
> because the evidence will still be there.
>
> If you are really intent on keeping the code out of the repository, another
> solution would be to remove commit access for this developer and instead
> have them email patches by running "svn diff | mail -s 'log message'",
> similar to how the Subversion project handles patches from non-committers.
We need a real diff format that could be read in to handle moves, property
changes, etc. Like what Tom Lord has been proposing to work on.
Emailing diffs is what I do now and it gets to be a pain. A nice automated
web page showing the proposed diff and having an accept or reject button
would sure be nice. This is similar to the Mailman interface to accepting
posts from non-members.
> I guess my main comment is if the code is bad just tuck it away someplace
> where it can't do any harm until it is fixed. If you don't want anyone to
> ever be able to see the code again, perhaps for public relations reasons,
> don't let it in the repository to begin with or restrict read access to
> that branch to those wearing protective equipment.
Unfortunately, that's not an option.
> Keeping a transaction open until you get around to reviewing a commit just
> doesn't feel right to me.
Having an XML or standardized diffs for review would work though.
That would be cool. Send the diff to an email address, it gets queued up
for review. A reviewer says ok, then a process tries to commit it. It may
conflict, but it wouldn't hurt the repository if it does.
Best,
Blair
--
Blair Zajac <bl...@orcaware.com>
Web and OS performance plots - http://www.orcaware.com/orca/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by Bruce Atherton <br...@callenish.com>.
At 04:45 PM 8/2/02 -0700, Blair Zajac wrote:
>I agree branching is good for this. However, this is more with working
>with a difficult person who hasn't been in a structured development
>group. I'd like to set up more structure to review modifications before
>they go into the tree and then just have more commits to clean up the
>previous one, even if you can get them to clean it up.
This sounds like a code promotion process. Subversion will definitely need
more tools for it, but you should be able to make it work with a bit of
scripting now. I don't agree that holding open the transaction until
approval is the right idea.
David's idea of a branch per issue does solve your problem. Or you could
put each developer on their own branch and only allow merging a revision to
the trunk when an approval had been "signed" by an authorized person using
whatever method:email, property change, merge to trunk by someone with
permissions, whatever. The scary code would still be in the repository
(where it should be IMO) but it does no harm. And as an added bonus, you
can tease the person about it in the future when they finally get a clue,
because the evidence will still be there.
If you are really intent on keeping the code out of the repository, another
solution would be to remove commit access for this developer and instead
have them email patches by running "svn diff | mail -s 'log message'",
similar to how the Subversion project handles patches from non-committers.
Kind of singles the individual out if they are the only one that has to do
this, though. I guess you could get around that by writing an automated
script that called "svn ci" for all email not arriving from that particular
developer.
I guess my main comment is if the code is bad just tuck it away someplace
where it can't do any harm until it is fixed. If you don't want anyone to
ever be able to see the code again, perhaps for public relations reasons,
don't let it in the repository to begin with or restrict read access to
that branch to those wearing protective equipment. Keeping a transaction
open until you get around to reviewing a commit just doesn't feel right to me.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by Blair Zajac <bl...@orcaware.com>.
I agree branching is good for this. However, this is more with working
with a difficult person who hasn't been in a structured development
group. I'd like to set up more structure to review modifications before
they go into the tree and then just have more commits to clean up the
previous one, even if you can get them to clean it up.
Truly, this isn't a SCM issue but a management issue. It's just that
management is harder than throwing technology at it :)
Best,
Blair
David Waite wrote:
>
> I could see this working via branching - the developers do not have
> commit access to the release branches, but can create new branches
> corresponding to known issues, and has a mechanism for signalling for
> review after the change has been made and tested (possibly after several
> commits).
>
> The person in charge of merging approved changes would probably need
> something more complex than a CGI, however - I would hope that the merge
> would then be tested before being commited to the release branch.
>
> -David Waite
>
> Blair Zajac wrote:
>
> >At my work, we have a review-commit model. I'd like to try something
> >like this that uses Subversion's txn support.
> >
> >1) Developer does work.
> >2) Developer commits.
> >3) Commit system notes that developer's commit needs review and
> > a) Keeps the txn in the database and quits.
> > b) Runs a modified commit-email.pl on the txn to send out a review
> > request.
> > c) Quits without deleting txn.
> >4) Somebody reviews the changes and say OKs it.
> >5) The manager goes to a CGI interface to the repos which lists all
> > the outstanding txn's and selects the txn to apply.
> >
> >This is nice because then you know the exact commit being applied to
> >the tree. Normally in CVS you do the review and the developer may have
> >made some more changes to the modification that were not reviewed and
> >checks that in.
> >
> >Is this possible? Would it work in this case?
> >
> >1) Developer 1's commit gets saved for review and later commit.
> >2) Developer 2 has special access and commits immediately.
> >
> >I'm guessing that invalidates 1's txn?
> >
> >Also, do the txn's store the time that they were created in
> >server time so they can be properly sorted? You don't want
> >client time, because that may be messed up.
> >
> >Best,
> >Blair
> >
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
--
Blair Zajac <bl...@orcaware.com>
Web and OS performance plots - http://www.orcaware.com/orca/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by David Waite <ma...@akuma.org>.
I could see this working via branching - the developers do not have
commit access to the release branches, but can create new branches
corresponding to known issues, and has a mechanism for signalling for
review after the change has been made and tested (possibly after several
commits).
The person in charge of merging approved changes would probably need
something more complex than a CGI, however - I would hope that the merge
would then be tested before being commited to the release branch.
-David Waite
Blair Zajac wrote:
>At my work, we have a review-commit model. I'd like to try something
>like this that uses Subversion's txn support.
>
>1) Developer does work.
>2) Developer commits.
>3) Commit system notes that developer's commit needs review and
> a) Keeps the txn in the database and quits.
> b) Runs a modified commit-email.pl on the txn to send out a review
> request.
> c) Quits without deleting txn.
>4) Somebody reviews the changes and say OKs it.
>5) The manager goes to a CGI interface to the repos which lists all
> the outstanding txn's and selects the txn to apply.
>
>This is nice because then you know the exact commit being applied to
>the tree. Normally in CVS you do the review and the developer may have
>made some more changes to the modification that were not reviewed and
>checks that in.
>
>Is this possible? Would it work in this case?
>
>1) Developer 1's commit gets saved for review and later commit.
>2) Developer 2 has special access and commits immediately.
>
>I'm guessing that invalidates 1's txn?
>
>Also, do the txn's store the time that they were created in
>server time so they can be properly sorted? You don't want
>client time, because that may be messed up.
>
>Best,
>Blair
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
RE: Re: Holding commits till review
Posted by Bill Tutt <ra...@lyra.org>.
> From: Wilfredo Sanchez [mailto:wsanchez@mit.edu]
>
> Writing these with SVN may actually be harder, though, since
branches
> are
> not really an SVN mechanism, and the scripts would have to figure out
what
> your
> branching tree structure (/branches/XXX or /project/branches/XXX ...?)
is
> before proceeding to make/find a branch by name. I guess it may have
to
> take
> full paths to branches.
In Subversion, the name of a branch is the full path to the branch.
>
> The problem with the idea of using transactions is that it's not at
all
> clear
> how you'd deal with the case where the code is denied pending some
changes.
> The developer then needs to edit the transanction (probably not
feasible)
> or
> create a new one. And what happens to the working copy when a
transaction
> is
> accepted but not commited? If it is edited further and another commit
> comes
> along but the first txn is still in limbo?
>
Actually, it's perfectly conceptually feasible to edit a Subversion
transaction, but we're certainly not going to write the code to do that
for 1.0. The data model supports this kind of stuff quite easily.
Bill
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Holding commits till review
Posted by Wilfredo Sanchez <ws...@mit.edu>.
At Apple, I implemented this with CVS using branches, and it works rather well.
I wrote some shell scripts to ease the process. The pain was in large projects
because branching, tagging, and merging take so long. With SVN, this would be a
lot smoother. The process is:
- cvs co proj; cd proj
- <edit away>
- <file a big describing your fix, if it's not in response to an existing bug>
- cvs-make-branch PR-<bug#>
(this tags the wc at PR-xxx-base, makes a branch PR-xxx, updates to that branch)
- cvs add/remove as needed
(this should work before the cvs-make-branch, but CVS ends up adding on the
mainline even though we think we're committing on the branch if you do it in
the wrong order)
- cvs commit, note the branch name in the bug's diagnosis
- Set the bug's state to Fixed (goes to review)
If the bug fix gets nixed, the bug goes back to the Analyze state. The developer
edits the branch to try again, or starts a new one PR-xxxA.
Then the branch noted in the bug gets merged in. Note that the person doing the
merge may not be the developer who made the fix. In fact, in the case of large
projects with many developers, one person merges all the bugs, coordinates with
developers in case of conflicts, and does a compile and test before committing the
merged branches to the mainline. Source gets tagged with a new version number....
All Darwin systems and Mac OS X with dev tools have the scripts in /usr/bin:
cvs-make-branch # Mark foo-base, make and update to new branch foo
cvs-merge-branch # Merge -j foo-base -j foo
cvs-diff-branch # Diff -r foo-base -r foo
cvs-view-diffs # Show diffs in FileMerge
Writing these with SVN may actually be harder, though, since branches are
not really an SVN mechanism, and the scripts would have to figure out what your
branching tree structure (/branches/XXX or /project/branches/XXX ...?) is
before proceeding to make/find a branch by name. I guess it may have to take
full paths to branches.
The problem with the idea of using transactions is that it's not at all clear
how you'd deal with the case where the code is denied pending some changes.
The developer then needs to edit the transanction (probably not feasible) or
create a new one. And what happens to the working copy when a transaction is
accepted but not commited? If it is edited further and another commit comes
along but the first txn is still in limbo?
-wsv
On Friday, August 2, 2002, at 06:52 PM, Blair Zajac wrote:
> 1) Developer does work.
> 2) Developer commits.
> 3) Commit system notes that developer's commit needs review and
> a) Keeps the txn in the database and quits.
> b) Runs a modified commit-email.pl on the txn to send out a review
> request.
> c) Quits without deleting txn.
> 4) Somebody reviews the changes and say OKs it.
> 5) The manager goes to a CGI interface to the repos which lists all
> the outstanding txn's and selects the txn to apply.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org