Hi, this question seems to arise periodically, so let's have a constructive discussion again about the issue once again.
The issue is being that a lot of people have direct commit access to the repository, and sooner or later some of them commit patches which introduce a regress in functionality.
I will use Magnus (GreatLord) again as an example. He was doing a good thing (as usual), but then somehow lost focus, and along with good fixes committed a breakage for FF2.0 installer (and all other apps using PatBlt functions). I had to (as usual!) spend the whole morning reviewing all his diffs, then regress-testing by bisection to find the guilty revision, then work out a better solution with him, and then come to agreement that the guilty commit could be just reverted for now, since it partly works anyway.
This takes up significant amount of my time, which could go into something more productive than just testing patches, finding way to unregress, and so on.
Especially I'm angry about this happening right when approaching 0.3.5 release.
Ideally (as I explained in #reactos today to blight_), I want the trunk itself be directly committable only by a very limited number of persons, and all developers should be committing to "some other place" at first, and then their patches merged by reviewers/testers (yes, manually, sometimes just reviewing may be enough to commit/ reject the patch, sometimes a thorough testing must be involved, maybe additional devs asked for a review).
blight_ called this as a "linux development model" with an intermediate branch - yes, maybe, why not?.
What I definately don't want to do is to scare developers away and make their life more complicated by need of sending text patches (this is just not convinient, and as seen on the example of Wine makes a lot of possible contributors going away).
I remember what Alex proposed (those who don't, may look up ros-dev ML archives with a similar topic).
Would there be any new prepositions? Maybe someone discovered a new CIS for SVN, or a decentralized VCS for patch submission and usual SVN for maintaining HEAD?
With time, developers number is only going to increase, so this problem will become more and more annoying.
With the best regards, Aleksey Bragin.
Yes well, if you had listened to my idea, you would've avoided all this:
1) Magnus would've had to create a bugzilla bug report for the 'issue' he was 'fixing' 1<- At this step, someone responsible for win32k should've gotten the bug report, and maybe had time to reject the patch 2) Magnus would've had to commit his patch to the PR-xxx branch (Which would've been created automatically server-side, behind Magnus' back) 2<- At this step, trunk would still be fine 3) At the end of the day, the integration suite would find the regression in PR-xxx, and remove it from the mergeset 3<- At this step, everyone is aware that Magnus' patch caused a regression, and the patch was removed from the mergeset 4) The integration suite commits the latest mergeset 4<- At this step, everyone's patches are in, trunk still boots, and Magnus's patch is to be reviewed.
But of course, you stubbornly continue to go forward without any sort of codebase management...
On 7-Jun-08, at 6:45 AM, Aleksey Bragin wrote:
Hi, this question seems to arise periodically, so let's have a constructive discussion again about the issue once again.
The issue is being that a lot of people have direct commit access to the repository, and sooner or later some of them commit patches which introduce a regress in functionality.
I will use Magnus (GreatLord) again as an example. He was doing a good thing (as usual), but then somehow lost focus, and along with good fixes committed a breakage for FF2.0 installer (and all other apps using PatBlt functions). I had to (as usual!) spend the whole morning reviewing all his diffs, then regress-testing by bisection to find the guilty revision, then work out a better solution with him, and then come to agreement that the guilty commit could be just reverted for now, since it partly works anyway.
This takes up significant amount of my time, which could go into something more productive than just testing patches, finding way to unregress, and so on.
Especially I'm angry about this happening right when approaching 0.3.5 release.
Ideally (as I explained in #reactos today to blight_), I want the trunk itself be directly committable only by a very limited number of persons, and all developers should be committing to "some other place" at first, and then their patches merged by reviewers/testers (yes, manually, sometimes just reviewing may be enough to commit/ reject the patch, sometimes a thorough testing must be involved, maybe additional devs asked for a review).
blight_ called this as a "linux development model" with an intermediate branch - yes, maybe, why not?.
What I definately don't want to do is to scare developers away and make their life more complicated by need of sending text patches (this is just not convinient, and as seen on the example of Wine makes a lot of possible contributors going away).
I remember what Alex proposed (those who don't, may look up ros-dev ML archives with a similar topic).
Would there be any new prepositions? Maybe someone discovered a new CIS for SVN, or a decentralized VCS for patch submission and usual SVN for maintaining HEAD?
With time, developers number is only going to increase, so this problem will become more and more annoying.
With the best regards, Aleksey Bragin. _______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
Best regards, Alex Ionescu
On Sat, Jun 7, 2008 at 10:13 AM, Alex Ionescu ionucu@videotron.ca wrote:
1<- At this step, someone responsible for win32k should've gotten the bug report, and maybe had time to reject the patch
This implies that module has an owner. Not every module does. Some people such as yourself and Fireball are adamant about reviewing what goes in and spotting breakages real quick. Other parts like the BSD code used in the network stack might not have the same level of review so step 1 could be a non-starter. I agree with all the other steps though, having branches, commit to the branch, run the test, integration suite does the rest of the magic and commits to the trunk everything should be good provided the system and tests are robust enough.
Thanks
The process doesn't depend on Step 1, but having it helps there -- it protects modules such as Win32k and Ntoskrnl a bit more than if the step wasn't there at all.
More importantly, it documents the process -- it leaves a paper trail, which is a lot better than today's typical "Revert after IRC discussion" commit logs...
Best regards, Alex Ionescu
On Sat, Jun 7, 2008 at 4:41 PM, Steven Edwards winehacker@gmail.com wrote:
On Sat, Jun 7, 2008 at 10:13 AM, Alex Ionescu ionucu@videotron.ca wrote:
1<- At this step, someone responsible for win32k should've gotten the bug report, and maybe had time to reject the patch
This implies that module has an owner. Not every module does. Some people such as yourself and Fireball are adamant about reviewing what goes in and spotting breakages real quick. Other parts like the BSD code used in the network stack might not have the same level of review so step 1 could be a non-starter. I agree with all the other steps though, having branches, commit to the branch, run the test, integration suite does the rest of the magic and commits to the trunk everything should be good provided the system and tests are robust enough.
Thanks
Steven Edwards
"There is one thing stronger than all the armies in the world, and that is an idea whose time has come." - Victor Hugo _______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
Hi!
On Sat, Jun 7, 2008 at 9:13 AM, Alex Ionescu ionucu@videotron.ca wrote:
Yes well, if you had listened to my idea, you would've avoided all this:
- Magnus would've had to create a bugzilla bug report for the 'issue'
he was 'fixing' 1<- At this step, someone responsible for win32k should've gotten the bug report, and maybe had time to reject the patch 2) Magnus would've had to commit his patch to the PR-xxx branch (Which would've been created automatically server-side, behind Magnus' back) 2<- At this step, trunk would still be fine 3) At the end of the day, the integration suite would find the regression in PR-xxx, and remove it from the mergeset 3<- At this step, everyone is aware that Magnus' patch caused a regression, and the patch was removed from the mergeset 4) The integration suite commits the latest mergeset 4<- At this step, everyone's patches are in, trunk still boots, and Magnus's patch is to be reviewed.
But of course, you stubbornly continue to go forward without any sort of codebase management...
Well,,, I guess if I was more assertive this would not been a problem....
I volunteered to be the Gdi rewrite leader, when I became the leader I was not the leader. Everyone else assumed they would do AS THY WILT and move on. Hay I tried to keep it under control I guess I'm just not assertive enough.... It's like fighting the undertow at high tide. I can't keep up with it......
Aleksey Bragin wrote:
Ideally (as I explained in #reactos today to blight_), I want the trunk itself be directly committable only by a very limited number of persons, and all developers should be committing to "some other place" at first, and then their patches merged by reviewers/testers (yes, manually, sometimes just reviewing may be enough to commit/ reject the patch, sometimes a thorough testing must be involved, maybe additional devs asked for a review).
blight_ called this as a "linux development model" with an intermediate branch - yes, maybe, why not?.
The worst case I see with doing this using SVN is that we have a lot of branches, which either depend on each other or are even incompatible to each other.
I remember what Alex proposed (those who don't, may look up ros-dev ML archives with a similar topic).
That development model sounds good, but I see a simple but annoying problem here: If we keep using SVN and a developer does a second patch to the same component before his first patch was committed, SVN's diff command will output a patch with the changes of the first and second patch.
Would there be any new prepositions? Maybe someone discovered a new CIS for SVN, or a decentralized VCS for patch submission and usual SVN for maintaining HEAD?
I recently read about Mercurial (Hg) and it sounds interesting. It's a very fast and decentralized VCS and would also solve the problems I mentioned above: We could have two repositories, one like the current trunk and another one only containing verified changes. Merging changes from one repository to the other one should be very easy with Hg. Also every developer would have his own repository when doing a checkout, so he can commit patches to it and later commit them all to the "playground trunk" or a branch or post a bunch of independent patch files to Bugzilla or wherever.
Unlike GIT, Hg also works quite well under Windows like SVN does: There exist command-line tools and TortoiseHg for Windows-Explorer integration. The only downside I see is the usage of Python, which might annoy developers who don't want to install (yet another) interpreter on their system.
Of course, the other way to work against these problems is focusing on automated regression testing. If we have a quite large test suite and could run that automatically, the regression Magnus introduced could probably be found as well without any human waste of time.
Best regards,
Colin
On Saturday 07 June 2008 10:24:42 am Colin Finck wrote:
Aleksey Bragin wrote:
Ideally (as I explained in #reactos today to blight_), I want the trunk itself be directly committable only by a very limited number of persons, and all developers should be committing to "some other place" at first, and then their patches merged by reviewers/testers (yes, manually, sometimes just reviewing may be enough to commit/ reject the patch, sometimes a thorough testing must be involved, maybe additional devs asked for a review).
blight_ called this as a "linux development model" with an intermediate branch - yes, maybe, why not?.
The worst case I see with doing this using SVN is that we have a lot of branches, which either depend on each other or are even incompatible to each other.
This is managed (now) in Linux by having a "next release" branch where all code that is ready to go into the next release is gathered, tested, etc... Not that Linus doesn't still pick up patches for the mainline kernel - it just helps resolve the merge problems between the trees.
DRH
-
Aleksey Bragin -
Alex Ionescu -
Colin Finck -
Daniel Hazelton -
James Tabor -
Steven Edwards