Lukas Ocilka write:
> Dne 8.6.2011 15:16, Lukas Ocilka napsal(a):
> > Code Reviews
> > - In the SLMS team, we've found out that all the commits should be
> >   reviewed by someone else. This helps to identify potential issues in
> >   time. All the commits are public anyway.
> > - Fix|Easy to Medium: Either someone is always assigned to review
> >   everyone's code (takes too much time) or developers usually check
> >   code of someone else only.
> 
> Martin, thank you for the reviews you do. They've already helped to find
> several bugs or possible issues even before they reached our users and
> made some harm.
> 
> From my POV, this looks very promising. Others will hopefully help with
> the reviews later ;)
> 
> Bye
> Lukas
> 
> 

Well, actually I think that we should connect it with git. Git allows to have 
official git repository and a lot of development git repository, so we can do 
it same way like kernel, wine or other projects.
see e.g. http://help.github.com/fork-a-repo/ and 
http://help.github.com/send-pull-requests/ I really like this workflow, as it 
allows to easy checkout your modified repository and then test it, comment each 
line.

So you can work in branches on different features, bug fixes etc ( in git 
really easy, I hope we have for non-git users presentation on workshop ), do it 
and then send pull request which is reviewed by authorized reviewer and if 
everything is fine, then accept it. If there is problems, you can discuss it 
under pull request and improve branch ( fix issues ) and send again.

Advantages:
- centralized place for reviews
- well defined group which is  responsible for accepting patches
- you review whole functionality, not just one commit, but whole set ( so you 
can check if it is tested, documented, API looks reasonable, code is readable, 
follow style guide etc. ). We can e.g. create process that module backup need 
to review it for module specific things ( as side effect he also know how 
module changed, so in case of vacation he know code ) and general reviewer 
check general rules.

Disadvantages:
- need some time, quick hackish solution will not be so quick and easy
- we need to somehow force use of for packages only code from official repo ( 
e.g. new services for OBS could help as, as we can have hudson which observe 
this repo and send to devel repo packages with recent changes )


I think we should discuss it now and what is more important decide it on 
workshop where is everyone and we should decide future direction of YaST.

Pepa


-- 
Josef Reidinger
Appliance Toolkit team
maintaining parts of webyast and SLMS
author of rubygems - studio_api and net_observer (coauthor)
-- 
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to