Thread: RE: [Codestriker-user] Feature request
Brought to you by:
sits
From: Matthew H. <mat...@wa...> - 2004-02-09 17:35:53
|
David, > > 1) Developers being able to create topics under a different=20 > > developer's e-mail, as well as the rights to delete topics. > > - Suggested solution =3D> Add rights related to logging into=20 > > CodeStriker > > Rights: > > Admin Rights: > > Delete topics > > Add, Edit, Delete, Modify rights of users > > > > Basic Rights (functionality): > > Create topics > > "Your e-mail address field" will be disabled and=20 > > automatically filled in with user login (user e-mail) > > > > <For now we could add users via SQL into the database, but=20 > it would be=20 > > nice to have a "Users" section to "Add, Edit, Delete,=20 > Change rights,=20 > > etc."> >=20 > Yes... I suspect we'll be working on this once we get 1.8.0=20 > out the door,=20 > this is something I am very keen to do. 1.8.0 will provide=20 > the capability=20 > to modify topic attributes, so having some form of security=20 > here will be=20 > useful, however 1.8.0 will also provide a complete audit=20 > trail of how a=20 > topic has been changed. We'll have something similar to how Bugzilla=20 > manages its users. >=20 Awesome! Sounds really great. I look forward to the 1.8.0 update. Thanks for the details of lots of interesting good stuff coming. :) > > 2) Multiple differential data for the same CVS file. The=20 > scenario came=20 > > up that developers need to be able to commit code multiple times to=20 > > correlate work with another developer, but be able to save all the=20 > > differences that they have made to a particular file over a=20 > period of=20 > > time before the modifications were reviewed. Other developers would=20 > > also be committing their changes, so the differences saved=20 > should only=20 > > include what the first developer changed. Hopefully the following=20 > > example illustrates. >=20 > This doesn't make any sense to me! Why can't you "fold" all of the=20 > differences together into one diff segment? This is how things are=20 > normally done in a Code reviewing situation, or when patch files are=20 > generated. Before generating a "cvs diff", you usually need=20 > to do a "cvs=20 > update -d" to ensure that your working copying is bought=20 > uptodate with=20 > what is in the repository. This will "merge" all changes=20 > together from=20 > other authors so that when you do a "cvs diff", all the=20 > changes are in one=20 > diff segment. >=20 > I don't think this answers your question... perhaps if you=20 > can give a more=20 > detailed example from a process perspective as to when you=20 > would ever want=20 > to generate a diff file with multiple diff segments for the=20 > same file. In=20 > all cases, they are usually merged together into a single segment,=20 > otherwise its more work for the reviewer - they'd want to see=20 > all changes=20 > for a single file in one place, not in multiple places. >=20 We may be going against the grain of normal review procedures, or maybe I'm just not adequately describing the situation. Definitely I am open to suggestion and learn from how other organizations conduct their reviews. Let me take another try at describing what the scenario is that we're confronting, and then let me know if this is still odd, or if it is still unclear. - 2 developers work on the same source file source file (append.test) contents: #This is a test test =3D 0 #To see how to append append =3D 0 #Differential information from different revisions diffs =3D 0 #Into one diff file file =3D 0 - Developer 1 begins to make changes #This is a test test =3D 0 #To see how to append append =3D 1 #Differential information from different revisions diffs =3D 0 #Into one diff file file =3D 0 - Developer 2 begins to make changes #This is a test test =3D 0 #To see how to append append =3D 0 #Differential information from different revisions diffs =3D 1 #Into one diff file file =3D 0 - Developer 1 performs an "update" to get latest changes before committing their own. (Potentially needs to fix any conflicts.) no updates - Developer 1 performs a "diff" on the modified file, and the output is parsed and redirected to a text file. (developer1.diff) Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.1 diff -u -r1.1 append.test --- append.test 6 Feb 2004 23:01:29 -0000 1.1 +++ append.test 6 Feb 2004 23:02:32 -0000 @@ -1,7 +1,7 @@ #This is a test test =3D 0 #To see how to append -append =3D 0 +append =3D 1 #Differential information from different revisions diffs =3D 0 #Into one diff file - Developer 1 commits changes creating revision 1.2 - Developer 2 performs an "update" to get latest changes before committing their own. (Potentially needs to fix any conflicts.) gets new changes from developer 1's commit #This is a test test =3D 0 #To see how to append append =3D 1 #Differential information from different revisions diffs =3D 1 #Into one diff file file =3D 0 - Developer 2 performs a "diff" on the modified file, and the output is parsed and redirected to a text file. (developer2.diff) Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.2 diff -u -r1.2 append.test --- append.test 6 Feb 2004 23:03:27 -0000 1.2 +++ append.test 6 Feb 2004 23:03:53 -0000 @@ -3,6 +3,6 @@ #To see how to append append =3D 1 #Differential information from different revisions -diffs =3D 0 +diffs =3D 1 #Into one diff file file =3D 0 - Developer 2 commits changes creating revision 1.3 - Developer 1 begins to make changes #This is a test test =3D 0 #To see how to append append =3D 1 #Differential information from different revisions diffs =3D 0 #Into one diff file file =3D 1 - Developer 2 begins to make changes #This is a test test =3D 1 #To see how to append append =3D 1 #Differential information from different revisions diffs =3D 1 #Into one diff file file =3D 0 - Developer 1 performs an "update" to get latest changes before committing their own. (Potentially needs to fix any conflicts.) #This is a test test =3D 0 #To see how to append append =3D 1 #Differential information from different revisions diffs =3D 1 #Into one diff file file =3D 1 - Developer 1 performs a "diff" on the modified file, and the output is parsed and redirected <concatenated this time> to a text file. (developer1.diff) Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.3 diff -u -r1.3 append.test --- append.test 6 Feb 2004 23:04:49 -0000 1.3 +++ append.test 6 Feb 2004 23:06:27 -0000 @@ -5,4 +5,4 @@ #Differential information from different revisions diffs =3D 1 #Into one diff file -file =3D 0 +file =3D 1 - Developer 1 commits changes creating revision 1.4 - Developer 2 performs an "update" to get latest changes before committing their own. (Potentially needs to fix any conflicts.) #This is a test test =3D 1 #To see how to append append =3D 1 #Differential information from different revisions diffs =3D 1 #Into one diff file file =3D 1 - Developer 2 performs a "diff" on the modified file, and the output is parsed and redirected <concatenated this time> to a text file. (developer2.diff) Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.4 diff -u -r1.4 append.test --- append.test 6 Feb 2004 23:05:27 -0000 1.4 +++ append.test 6 Feb 2004 23:05:34 -0000 @@ -1,5 +1,5 @@ #This is a test -test =3D 0 +test =3D 1 #To see how to append append =3D 1 #Differential information from different revisions - Developer 2 commits changes creating revision 1.5 - Developer 1's diff file (developer1.diff) should look like the following: Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.1 diff -u -r1.1 append.test --- append.test 6 Feb 2004 23:01:29 -0000 1.1 +++ append.test 6 Feb 2004 23:02:32 -0000 @@ -1,7 +1,7 @@ #This is a test test =3D 0 #To see how to append -append =3D 0 +append =3D 1 #Differential information from different revisions diffs =3D 0 #Into one diff file Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.3 diff -u -r1.3 append.test --- append.test 6 Feb 2004 23:04:49 -0000 1.3 +++ append.test 6 Feb 2004 23:06:27 -0000 @@ -5,4 +5,4 @@ #Differential information from different revisions diffs =3D 1 #Into one diff file -file =3D 0 +file =3D 1 - Developer 2's diff file (developer2.diff) should look like the following: Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.2 diff -u -r1.2 append.test --- append.test 6 Feb 2004 23:03:27 -0000 1.2 +++ append.test 6 Feb 2004 23:03:53 -0000 @@ -3,6 +3,6 @@ #To see how to append append =3D 1 #Differential information from different revisions -diffs =3D 0 +diffs =3D 1 #Into one diff file file =3D 0 Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.4 diff -u -r1.4 append.test --- append.test 6 Feb 2004 23:05:27 -0000 1.4 +++ append.test 6 Feb 2004 23:05:34 -0000 @@ -1,5 +1,5 @@ #This is a test -test =3D 0 +test =3D 1 #To see how to append append =3D 1 #Differential information from different revisions - Now that both tasks have been completed, both developers would like to have each of their individual work reviewed. - Each developer creates a separate CodeStriker topic, and uploads/attaches their respective diff files. <Problem:> When a topic is created, only a single file entry is created for the "Contents:" list which is associated with the first encounter of the filename in the uploaded diff file. (i.e. Index: append.test) Subsequent entries of the same file are not associated with their respective revisions. All subsequent diff entries for an identified file entry is associated with the revision of the first encountered file entry. The "Parallel" and "non-Parallel?" views feature is also broken from this kind of diff file structure. <Possible Solution:>When parsing the uploaded diff file, create a new entry in the "Contents:" list for each file entry encountered in the diff file. This should also preserve revisioning and maintain the current "Parallel" and non-"Parallel" view functionality. I hope this explains what is happening more clearly. Is there any portion of this scenario that seems wrong/weird/unclear? Again, I'm open to suggestion and input. :) Thanks, Matthew=20 =20 *************************************=20 This e-mail may contain privileged or confidential material intended for = the named recipient only.=20 If you are not the named recipient, delete this message and all = attachments.=20 Unauthorized reviewing, copying, printing, disclosing, or otherwise = using information in this e-mail is prohibited.=20 We reserve the right to monitor e-mail sent through our network. =20 *************************************=20 |
From: Kelly F. H. <kf...@mq...> - 2004-02-10 13:11:21
|
> -----Original Message----- > From: David Sitsky [mailto:si...@us...] > Sent: Tuesday, February 10, 2004 4:16 AM > To: Matthew Hailstone; cod...@li... > Subject: Re: [Codestriker-user] Feature request >=20 > > - Now that both tasks have been completed, both developers would like to > > have each of their individual work reviewed. >=20 > This is the problem. Codestriker is designed to be a "pre-commit" step. >=20 > The usual scenario with development teams is that unreviewed code is _not_ > allowed to be committed into the CVS repository. Once code has been > reviewed, then the developer is free to commit it in. >=20 > The motivation for this is that a bad commit can impact the whole > development > team, resulting in potentially a lot of wasted time for bad commits. Hi David, Just FYI, we don't do development this way, nor does any development organization that I've been a part of for the last 20 years. We do projects in branches, have the branches reviewed, do unit test, THEN merge it to the main line of development. We want our developers to be able to do a commit at any time (and urge them to do it at least daily) to prevent mishaps due to power outages, drive failure, etc. This has paid off more than once! The other issue arises when you're working on a change that affects several different platforms (aix, solaris and windows for instance), you need to be able to commit, so that you can check the code out on the other machines to do unit testing. -Kelly >=20 > In your scenario, there should be no commits by developer 1 or 2. Once > they > have finished their "work", then they produce a "cvs diff" which is then > reviewed in Codestriker. >=20 > There is no problem with both reviewers doing this in parallel - CVS is > designed to handle this merges and so forth. >=20 > The challenge is to define sensible units of work, which aren't too large > to > review, but software teams become very experienced at doing this very > quickly. >=20 > Cheers, > David >=20 >=20 > ------------------------------------------------------- > The SF.Net email is sponsored by EclipseCon 2004 > Premiere Conference on Open Tools Development and Integration > See the breadth of Eclipse activity. February 3-5 in Anaheim, CA. > http://www.eclipsecon.org/osdn > _______________________________________________ > Codestriker-user mailing list > Cod...@li... > https://lists.sourceforge.net/lists/listinfo/codestriker-user |
From: David S. <si...@us...> - 2004-02-10 21:22:39
|
> Hi David, > Just FYI, we don't do development this way, nor does any > development organization that I've been a part of for the last 20 years. > We do projects in branches, have the branches reviewed, do unit test, > THEN merge it to the main line of development. We want our developers > to be able to do a commit at any time (and urge them to do it at least > daily) to prevent mishaps due to power outages, drive failure, etc. > This has paid off more than once! The other issue arises when you're > working on a change that affects several different platforms (aix, > solaris and windows for instance), you need to be able to commit, so > that you can check the code out on the other machines to do unit > testing. Hi Kelly, I guess the key message here is that with your process, each developer has their own separate _private_ branch, so even if they do daily commits to their private branch, there is no impact to the rest of the development team. With your process, there is still a way once the work has been completed, to generate a diff that can be reviewed, and then merged with the main branch. With the process described by Matthew, unreviewed code from more than one developer is being committed into a branch/main trunk, which I think is a big problem. The basic message is unreviewed code should never be committed to a "public" branch that other developers are using. -- Cheers, David |
From: Kelly F. H. <kf...@mq...> - 2004-02-10 21:25:02
|
> -----Original Message----- > From: David Sitsky [mailto:si...@us...] > Sent: Tuesday, February 10, 2004 3:22 PM > To: Kelly F. Hickel; Matthew Hailstone; codestriker- > us...@li... > Subject: Re: [Codestriker-user] Feature request >=20 > > Hi David, > > Just FYI, we don't do development this way, nor does any > > development organization that I've been a part of for the last 20 years. > > We do projects in branches, have the branches reviewed, do unit test, > > THEN merge it to the main line of development. We want our developers > > to be able to do a commit at any time (and urge them to do it at least > > daily) to prevent mishaps due to power outages, drive failure, etc. > > This has paid off more than once! The other issue arises when you're > > working on a change that affects several different platforms (aix, > > solaris and windows for instance), you need to be able to commit, so > > that you can check the code out on the other machines to do unit > > testing. >=20 > Hi Kelly, >=20 > I guess the key message here is that with your process, each developer has > their own separate _private_ branch, so even if they do daily commits to > their private branch, there is no impact to the rest of the development > team. With your process, there is still a way once the work has been > completed, to generate a diff that can be reviewed, and then merged with > the main branch. >=20 > With the process described by Matthew, unreviewed code from more than one > developer is being committed into a branch/main trunk, which I think is a > big problem. >=20 > The basic message is unreviewed code should never be committed to a > "public" branch that other developers are using. David, While I (in theory) agree with your last statement, we do have multiple developers working on projects (not always, but often), and so, they do share a branch. They are fully authorized to beat each other with a wet noodle if one of them breaks the other's code (;>), but it does happen. We do the reviews at the end, and sometimes the reviews cause changes, so we need to review it again, requiring a new codestriker topic, instead of just updating the existing one. -Kelly7 >=20 > -- > Cheers, > David |
From: David S. <si...@us...> - 2004-02-10 22:24:56
|
> > The basic message is unreviewed code should never be committed to a > > "public" branch that other developers are using. > > David, > While I (in theory) agree with your last statement, we do have > multiple developers working on projects (not always, but often), and so, > they do share a branch. They are fully authorized to beat each other > with a wet noodle if one of them breaks the other's code (;>), but it > does happen. We do the reviews at the end, and sometimes the reviews > cause changes, so we need to review it again, requiring a new > codestriker topic, instead of just updating the existing one. Ok, but in terms of reviewing code going into the main branch, you are happy to review it as an ordinary diff file? What Matthew is proposing is to create a diff file that contains separate sections for each developer. -- Cheers, David |
From: Matthew H. <mat...@wa...> - 2004-02-10 21:51:32
|
David, I understand your suggestion. I agree with you that on the whole, code should be reviewed before committed. Unfortunately, I am still faced with the problem because other managers do not agree with that philosophy in all cases. Most of the time that will be the case, but because our processes are more time driven than event driven I'm coming up towards a brick wall with the scenario I described earlier. I am really excited about incorporating CodeStriker into our process, and except for this problem, everything is really going well. :) Here are the diff files that would be created in my scenario and my proposal: - Developer 1's diff file (developer1.diff) should look like the following: Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.1 diff -u -r1.1 append.test --- append.test 6 Feb 2004 23:01:29 -0000 1.1 +++ append.test 6 Feb 2004 23:02:32 -0000 @@ -1,7 +1,7 @@ #This is a test test =3D 0 #To see how to append -append =3D 0 +append =3D 1 #Differential information from different revisions diffs =3D 0 #Into one diff file Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.3 diff -u -r1.3 append.test --- append.test 6 Feb 2004 23:04:49 -0000 1.3 +++ append.test 6 Feb 2004 23:06:27 -0000 @@ -5,4 +5,4 @@ #Differential information from different revisions diffs =3D 1 #Into one diff file -file =3D 0 +file =3D 1 - Developer 2's diff file (developer2.diff) should look like the following: Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.2 diff -u -r1.2 append.test --- append.test 6 Feb 2004 23:03:27 -0000 1.2 +++ append.test 6 Feb 2004 23:03:53 -0000 @@ -3,6 +3,6 @@ #To see how to append append =3D 1 #Differential information from different revisions -diffs =3D 0 +diffs =3D 1 #Into one diff file file =3D 0 Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.4 diff -u -r1.4 append.test --- append.test 6 Feb 2004 23:05:27 -0000 1.4 +++ append.test 6 Feb 2004 23:05:34 -0000 @@ -1,5 +1,5 @@ #This is a test -test =3D 0 +test =3D 1 #To see how to append append =3D 1 #Differential information from different revisions - Now that both tasks have been completed, both developers would like to have each of their individual work reviewed. - Each developer creates a separate CodeStriker topic, and uploads/attaches their respective diff files. <Problem:> When a topic is created, only a single file entry is created for the "Contents:" list which is associated with the first encounter of the filename in the uploaded diff file. (i.e. Index: append.test) Subsequent entries of the same file are not associated with their respective revisions. All subsequent diff entries for an identified file entry is associated with the revision of the first encountered file entry. The "Parallel" and "non-Parallel?" views feature is also broken from this kind of diff file structure. <Possible Solution:>When parsing the uploaded diff file, create a new entry in the "Contents:" list for each file entry encountered in the diff file. This should also preserve revisioning and maintain the current "Parallel" and non-"Parallel" view functionality. If you try these kind of diff files, do you see the same problems happening? Thanks, :) Matthew=20 =20 *************************************=20 This e-mail may contain privileged or confidential material intended for = the named recipient only.=20 If you are not the named recipient, delete this message and all = attachments.=20 Unauthorized reviewing, copying, printing, disclosing, or otherwise = using information in this e-mail is prohibited.=20 We reserve the right to monitor e-mail sent through our network. =20 *************************************=20 |
From: Matthew H. <mat...@wa...> - 2004-02-10 23:21:42
|
> Ok, but in terms of reviewing code going into the main=20 > branch, you are=20 > happy to review it as an ordinary diff file? >=20 Correct. Doing a diff on a modified file, producing the unified diff output and redirecting that output to a file, then uploading that file to a topic in Codestriker should accomplish this. > What Matthew is proposing is to create a diff file that=20 > contains separate=20 > sections for each developer. >=20 Your comment about a "diff file that contains separate sections for each developer" concerns me though. Basically, the only changes that I see that Codestriker would have to make is how it parses the uploaded diff file. Currently each developer needs to: - Create a diff file - Create a Codestriker topic (upload their own diff file) This would remain the same with my scenario. The only difference would be that the diff file uploaded in our scenario would contain multiple entries of the same file with different revision diffs. Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.1 diff -u -r1.1 append.test --- append.test 6 Feb 2004 23:01:29 -0000 1.1 +++ append.test 6 Feb 2004 23:02:32 -0000 @@ -1,7 +1,7 @@ #This is a test test =3D 0 #To see how to append -append =3D 0 +append =3D 1 #Differential information from different revisions diffs =3D 0 #Into one diff file Index: append.test =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsrep/test/append.test,v retrieving revision 1.3 diff -u -r1.3 append.test --- append.test 6 Feb 2004 23:04:49 -0000 1.3 +++ append.test 6 Feb 2004 23:06:27 -0000 @@ -5,4 +5,4 @@ #Differential information from different revisions diffs =3D 1 #Into one diff file -file =3D 0 +file =3D 1 When the diff file is uploaded currently, the 1.3 diff is put under the 1.1 listing of the "append.test" file. I am suggesting that when the diff file is uploaded and parsed, that 2 listings are created for the "append.test" file (one for each occurrence in the diff file which will be different revision diffs). Each developer would still have to create their own diff file and create their own Codestriker topic. BTW, this has been a good exercise for me to evaluate myself in how well I'm communicating the problem. I hope I'm making more sense and not nonsense. Thanks! :) Matthew=20 =20 *************************************=20 This e-mail may contain privileged or confidential material intended for = the named recipient only.=20 If you are not the named recipient, delete this message and all = attachments.=20 Unauthorized reviewing, copying, printing, disclosing, or otherwise = using information in this e-mail is prohibited.=20 We reserve the right to monitor e-mail sent through our network. =20 *************************************=20 |
From: David S. <si...@us...> - 2004-02-11 22:46:23
|
> Basically, the only changes that I see that Codestriker would have to > make is how it parses the uploaded diff file. > > Currently each developer needs to: > - Create a diff file > - Create a Codestriker topic (upload their own diff file) > > This would remain the same with my scenario. The only difference would > be that the diff file uploaded in our scenario would contain multiple > entries of the same file with different revision diffs. There is an assumption in the Codestriker database that there is only a single diff change per filename in the patch file. Patch files (and diffs files from all SCM systems) have this property. I still maintain it would be confusing for a reviewer to see a diff chunk for more than one file, and then to see another diff chunk for the same file. The diff files you are talking about would have to be created by a custom tool, they can'tbe created by the SCM system. A reviewer wants to see the cumulative code changes in one place, not in several places which would require them to mentally "merge" them together. Your provided example is very simple, and mental merging here is easy, but in a real-life scenario, I think it would be frustrating and error prone, especially if the second diff chunk makes changes made on the same lines of the first diff chunk. What about the process Kelly describes? Does that suit your requirements better? Another option is before a developer does a commit for unreviewed code, they still post a Codestriker topic, but it gets reviewed at a future time. That way, if your process needs to commit code in the SCM now, you can still commit it, and it can get reviewed later, as the topic has been created. -- Cheers, David |
From: Kelly F. H. <kf...@mq...> - 2004-02-11 02:54:54
|
> -----Original Message----- > From: David Sitsky [mailto:si...@us...] > Sent: Tuesday, February 10, 2004 4:24 PM > To: Kelly F. Hickel; Matthew Hailstone; codestriker- > us...@li... > Subject: Re: [Codestriker-user] Feature request >=20 >=20 > > > The basic message is unreviewed code should never be committed to a > > > "public" branch that other developers are using. > > > > David, > > While I (in theory) agree with your last statement, we do have > > multiple developers working on projects (not always, but often), and so, > > they do share a branch. They are fully authorized to beat each other > > with a wet noodle if one of them breaks the other's code (;>), but it > > does happen. We do the reviews at the end, and sometimes the reviews > > cause changes, so we need to review it again, requiring a new > > codestriker topic, instead of just updating the existing one. >=20 > Ok, but in terms of reviewing code going into the main branch, you are > happy to review it as an ordinary diff file? >=20 > What Matthew is proposing is to create a diff file that contains separate > sections for each developer. Ahh, I missed that. No, what I want (which of course, translates to "the right thing" ;>) is to see all the chages on the branch rolled into one review screen. Ideally, each time I visited it, it would refresh to the latest set of changes on the branch. >=20 > -- > Cheers, > David See ya! -Kelly |
From: Matthew H. <mat...@wa...> - 2004-02-11 23:24:43
|
David, Your suggestion is what my secondary plan of action was going to be. The developers will just need to create a topic for each time that they commit, or do a diff between two revisions (beginning and end) and upload that file (notwithstanding potential changes from other developers will appear in the diff). This will have to be acceptable. :) Kelly's process of committing to particular branches would really break our current process. Not that your model isn't good, Kelly. It is just different than our very established way of branching and producing deliverables. :) Thanks for the discussion. It's interesting to learn how other organizations carry out the code review process. I look forward to 1.8.0! :) Matthew > There is an assumption in the Codestriker database that there=20 > is only a=20 > single diff change per filename in the patch file. Patch=20 > files (and diffs=20 > files from all SCM systems) have this property. I still=20 > maintain it would=20 > be confusing for a reviewer to see a diff chunk for more than=20 > one file,=20 > and then to see another diff chunk for the same file. The=20 > diff files you=20 > are talking about would have to be created by a custom tool,=20 > they can'tbe=20 > created by the SCM system. >=20 > A reviewer wants to see the cumulative code changes in one=20 > place, not in=20 > several places which would require them to mentally "merge"=20 > them together. =20 > Your provided example is very simple, and mental merging here=20 > is easy, but=20 > in a real-life scenario, I think it would be frustrating and=20 > error prone,=20 > especially if the second diff chunk makes changes made on the=20 > same lines=20 > of the first diff chunk. >=20 > What about the process Kelly describes? Does that suit your=20 > requirements=20 > better? Another option is before a developer does a commit=20 > for unreviewed=20 > code, they still post a Codestriker topic, but it gets reviewed at a=20 > future time. That way, if your process needs to commit code=20 > in the SCM=20 > now, you can still commit it, and it can get reviewed later,=20 > as the topic=20 > has been created. >=20 > --=20 > Cheers, > David >=20 >=20 =20 *************************************=20 This e-mail may contain privileged or confidential material intended for = the named recipient only.=20 If you are not the named recipient, delete this message and all = attachments.=20 Unauthorized reviewing, copying, printing, disclosing, or otherwise = using information in this e-mail is prohibited.=20 We reserve the right to monitor e-mail sent through our network. =20 *************************************=20 |
From: Dan P. <dan...@al...> - 2005-07-28 20:03:45
|
Comments below... David Sitsky wrote: > On Thu, 28 Jul 2005 00:26, cod...@sp... wrote: > >> It would be nice to have in the config file a value to set the valid >> open states, so that when I click on List all open topics, it would >> list all valid open states. > > > > Have you seen the @readonly_states config variable? In some sense, what you are asking for is the inverse of this. > > >> Do you think that feature would be useful to other? As of now, I >> modified the template to put [0, 3] to display all open topics. > > > > Maybe you can make the code check if the state is not in @readonly_states, to get the same/similar effect? This would certainly be possible, but I'm not sure it is what everyone would want. Some installations (probably ours, for one, though it's probably not a big deal) would want the 'List open topics' link to list only topics in state 'open' and not ones in state 'Reviewed', even though both are not 'read-only' states (i.e. topics in either state can be edited). > > Failing that - perhaps you could defined a @open_states config variable. > I would vote for this option. I'm assuming it would control only the states that get displayed when you click the 'List open states' link, and would be different from @readonly_states. |
From: David S. <si...@us...> - 2004-02-10 10:32:35
|
> - Now that both tasks have been completed, both developers would like to > have each of their individual work reviewed. This is the problem. Codestriker is designed to be a "pre-commit" step. The usual scenario with development teams is that unreviewed code is _not_ allowed to be committed into the CVS repository. Once code has been reviewed, then the developer is free to commit it in. The motivation for this is that a bad commit can impact the whole development team, resulting in potentially a lot of wasted time for bad commits. In your scenario, there should be no commits by developer 1 or 2. Once they have finished their "work", then they produce a "cvs diff" which is then reviewed in Codestriker. There is no problem with both reviewers doing this in parallel - CVS is designed to handle this merges and so forth. The challenge is to define sensible units of work, which aren't too large to review, but software teams become very experienced at doing this very quickly. Cheers, David |