You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@subversion.apache.org by David Garamond <li...@zara.6.isreserved.com> on 2003/11/29 12:37:32 UTC

code reviewing and test enforcement?

We're trying to use subversion instead of aegis for recent projects. For
no particular reason, really, just to try out new stuffs. Also,
subversion has tools like TortoiseSVN which makes it convenient to use.

Using subversion, for projects involving several developers and a code
reviewer, how might one implement obligatory testing (i.e., a test
script has to be run first and succeeds before commit can continue) and
code reviewing (i.e., when a developer commit a change, it goes to the
code reviewer first; if the reviewer rejects the code, nothing is
committed). Developers use "svn diff" + email the reviewer instead of
"svn commit"? Use two repositories?

-- 
dave



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

Re: code reviewing and test enforcement?

Posted by John Szakmeister <jo...@szakmeister.net>.
On Sunday 30 November 2003 08:18, Ben Collins-Sussman wrote:
> On Sun, 2003-11-30 at 05:45, John Szakmeister wrote:
> > I thought the reason for 'svnadmin lstxns/rmtxns' was because it is
> > possible to have a transaction lying around.  If this is no longer the
> > case, why do we still have these commands?
>
> Because
>
>  1) bugs in libsvn_repos might accidentally leave a transaction lying
> around after an error,
>
>  2) if someone uses libsvn_fs directly, then it's definitely possible to
> have a long-lived transaction.

Ahhk yes, I forget that we allow people to use libsvn_fs directly.  Thanks for 
the enlightenment.

-John


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

Re: code reviewing and test enforcement?

Posted by David Garamond <li...@zara.6.isreserved.com>.
Ben Collins-Sussman wrote:
> On Sun, 2003-11-30 at 05:45, John Szakmeister wrote:
>>I thought the reason for 'svnadmin lstxns/rmtxns' was because it is possible 
>>to have a transaction lying around.  If this is no longer the case, why do we 
>>still have these commands? 
> 
> Because
> 
>  1) bugs in libsvn_repos might accidentally leave a transaction lying
> around after an error,
> 
>  2) if someone uses libsvn_fs directly, then it's definitely possible to
> have a long-lived transaction.

Thanks for all the answers and discussion. I believe our team can work 
out a simple workflow over svn just fine.

-- 
dave



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

Re: code reviewing and test enforcement?

Posted by Ben Collins-Sussman <su...@collab.net>.
On Sun, 2003-11-30 at 05:45, John Szakmeister wrote:

> I thought the reason for 'svnadmin lstxns/rmtxns' was because it is possible 
> to have a transaction lying around.  If this is no longer the case, why do we 
> still have these commands? 

Because

 1) bugs in libsvn_repos might accidentally leave a transaction lying
around after an error,

 2) if someone uses libsvn_fs directly, then it's definitely possible to
have a long-lived transaction.




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

Re: code reviewing and test enforcement?

Posted by John Szakmeister <jo...@szakmeister.net>.
On Saturday 29 November 2003 23:53, Ben Collins-Sussman wrote:
> On Sat, 2003-11-29 at 22:21, Brad Appleton wrote:
> > On Sat, Nov 29, 2003 at 08:05:08AM -0600, Ben Collins-Sussman wrote:
> > > If you want the SCM system to enforce code review and unit tests,
> > > you'll have to write a whole new "workflow" system on top of
> > > Subversion... it's not part of Subversion itself.
> >
> > Isn't what the OP is asking for simply the Aegis-like feature of
> > having a pre-commit "hook"? I perhaps tried to read too much into the
> > posting, but I got the impression he simply wanted a way to be able to
> > have his own pre-commit hook execute and be able to "do its thing" and
> > then return a success/failure result that the commit command would
> > then abort/proceed based upon its status.
>
> Sure... but how would you design a "pre-commit hook" to
>
>   1) verify that a unit-test was committed with the change
>   2) verify that the unit-test passes
>
> ?
> Because that's what Aegis does.  It's not a trivial thing.  Definitely
> beyond the scope of "core" Subversion at the moment.

Agreed.

> Beyond that, our repository hook system currently doesn't allow "long
> lived" transactions;  if the pre-commit hook fails, I believe it deletes
> the transaction, and if it succeeds, I believe it commits the
> transaction.  To make a transaction last indefinitely (so a manager can
> review it) requires writing some new code wrappers around the public
> filesystem API.   Again, not a trivial thing.

I thought the reason for 'svnadmin lstxns/rmtxns' was because it is possible 
to have a transaction lying around.  If this is no longer the case, why do we 
still have these commands? 

-John


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

Re: code reviewing and test enforcement?

Posted by Brad Appleton <br...@bradapp.net>.
On Sat, Nov 29, 2003 at 10:53:22PM -0600, Ben Collins-Sussman wrote:
> > Isn't what the OP is asking for simply the Aegis-like feature of
> > having a pre-commit "hook"? I perhaps tried to read too much into the
> > posting, but I got the impression he simply wanted a way to be able to
> > have his own pre-commit hook execute and be able to "do its thing" and
> > then return a success/failure result that the commit command would
> > then abort/proceed based upon its status.
> 
> Sure... but how would you design a "pre-commit hook" to
> 
>   1) verify that a unit-test was committed with the change
>   2) verify that the unit-test passes

That my (or the OPs) problem to solve. All I need Subversion
to do is give me the hook. The rest is up to me. There is no
need/requirement for any long-lived transaction here. Whether
it is trivial or not is entirely up to me and how I choose to
enforce it. I've seen this sort of thing solved with extremely
sophisticated and complex mechanisms that implement additional
workflow and nested transactions.

I've also seen this sort of thing implemented with extremely
simple formatting in a single text file and some source tree
standards for where a set of tests should go and how they
should be named and how to know which tests correspond to which
"files" or modules, and where to write the results so that a
pre-commit-hook can check all of it without a ton of effort
(most of the "effort" is in getting the team to agree to 
the standards/conventions) so that the pre-commit hook can:
 - Look at the set of files in the to-be-committed change-set
 - Compute the set of corresponding test-file names using the standard
 - Use the naming convention to map class/method/model/whatever
   names to corresponding test names/numbers and make sure they are there
 - Run the tests and/or look for the test-results as "published"
   in some text file in a known location
 - If desired look for a manager "signoff" using whatever desired
   mechanism (MD5, or whatever) or even use a "property" of some
   sort and check the history/event logs to verify the right
   manager or "group" signed off (or what have you)

It aint perfect, but its "good enough" and its simple enough. If it
fails, I can write the failure reason to a known file, even write
some "status/state" info and generate some email or talk to my
tracking-tool to fire off a signoff workflow, or simply consult the
tracking tool for the signoff and look for the corresponding state
there.

Whatever I do, that is of course up to me. I just need the pre-commit
hook and the failure/success return. There is no need to assume a
broader requirement for long-lived transactions or nested transactions
or any of that stuff. Not that those aren't nice to have for other
reasons of course - but I don't see those being asked for here. All
thats being asked for is a way to get assistance from subversion 
in enforcing a process, and all thats needed is the "hook" between
however I choose to implement it myself and the commit-operation.

-- 
Brad Appleton <br...@bradapp.net> www.bradapp.net
  Software CM Patterns (www.scmpatterns.com)
   Effective Teamwork, Practical Integration
"And miles to go before I sleep." -- Robert Frost

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

Re: code reviewing and test enforcement?

Posted by Ben Collins-Sussman <su...@collab.net>.
On Sat, 2003-11-29 at 22:21, Brad Appleton wrote:
> On Sat, Nov 29, 2003 at 08:05:08AM -0600, Ben Collins-Sussman wrote:
> > If you want the SCM system to enforce code review and unit tests, you'll
> > have to write a whole new "workflow" system on top of Subversion... it's
> > not part of Subversion itself.
> 
> Isn't what the OP is asking for simply the Aegis-like feature of
> having a pre-commit "hook"? I perhaps tried to read too much into the
> posting, but I got the impression he simply wanted a way to be able to
> have his own pre-commit hook execute and be able to "do its thing" and
> then return a success/failure result that the commit command would
> then abort/proceed based upon its status.

Sure... but how would you design a "pre-commit hook" to

  1) verify that a unit-test was committed with the change
  2) verify that the unit-test passes
 
?  
Because that's what Aegis does.  It's not a trivial thing.  Definitely
beyond the scope of "core" Subversion at the moment.

Beyond that, our repository hook system currently doesn't allow "long
lived" transactions;  if the pre-commit hook fails, I believe it deletes
the transaction, and if it succeeds, I believe it commits the
transaction.  To make a transaction last indefinitely (so a manager can
review it) requires writing some new code wrappers around the public
filesystem API.   Again, not a trivial thing.




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

Re: code reviewing and test enforcement?

Posted by Brad Appleton <br...@bradapp.net>.
On Sat, Nov 29, 2003 at 08:05:08AM -0600, Ben Collins-Sussman wrote:
> If you want the SCM system to enforce code review and unit tests, you'll
> have to write a whole new "workflow" system on top of Subversion... it's
> not part of Subversion itself.

Isn't what the OP is asking for simply the Aegis-like feature of having a pre-commit "hook"? I perhaps tried to read too much into the posting, but I got the impression he simply wanted a way to be able to have his own pre-commit hook execute and be able to "do its thing" and then return a success/failure result that the commit command would then abort/proceed based upon its status.

-- 
Brad Appleton <br...@bradapp.net> www.bradapp.net
  Software CM Patterns (www.scmpatterns.com)
   Effective Teamwork, Practical Integration
"And miles to go before I sleep." -- Robert Frost

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

Re: code reviewing and test enforcement?

Posted by Ben Collins-Sussman <su...@collab.net>.
On Sat, 2003-11-29 at 06:37, David Garamond wrote:
> We're trying to use subversion instead of aegis for recent projects. For
> no particular reason, really, just to try out new stuffs. Also,
> subversion has tools like TortoiseSVN which makes it convenient to use.
> 
> Using subversion, for projects involving several developers and a code
> reviewer, how might one implement obligatory testing (i.e., a test
> script has to be run first and succeeds before commit can continue) and
> code reviewing (i.e., when a developer commit a change, it goes to the
> code reviewer first; if the reviewer rejects the code, nothing is
> committed). Developers use "svn diff" + email the reviewer instead of
> "svn commit"? Use two repositories?

If you want the SCM system to enforce code review and unit tests, you'll
have to write a whole new "workflow" system on top of Subversion... it's
not part of Subversion itself.

Subversion certainly has the foundation laid out for you... in theory, a
commit transaction can live indefinitely, and a reviewer can examine it
before committing it.  A simpler solution is to have people commit every
change to a real branch directory, and then have an "integrator" merge
each branch to the trunk, and then delete the branch.



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