Development process - once again

[Aleksey Bragin]

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.

[Alex Ionescu]

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(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev 
Best regards,
Alex Ionescu

[Steven Edwards]

On Sat, Jun 7, 2008 at 10:13 AM, Alex Ionescu <ionucu(a)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

[Alex Ionescu]

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(a)gmail.com> wrote:
>   On Sat, Jun 7, 2008 at 10:13 AM, Alex Ionescu
> <ionucu(a)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(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>

[James Tabor]

Hi!

On Sat, Jun 7, 2008 at 9:13 AM, Alex Ionescu <ionucu(a)videotron.ca> wrote:
>   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...
>  
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......

[Colin Finck]

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

[Daniel Hazelton]

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

-- 
Dialup is like pissing through a pipette. Slow and excruciatingly painful.