[libvirt] Suggestion on commit rules when fixing breakages
As the number of compilation options and platform grows, it gets more difficult for a commiter to always ensure one chunk of code won't give a problem in a different situation. To try to lower the cost of maintaining the protability I would suggest the following rule for commit: - if a recently commited patch breaks compilation on a platform or for a given driver then it's fine to commit a minimal fix directly without getting the review feedback first - similary if make check or make syntax-chek breaks, if there is an obvious fix, it's fine to commit immediately Note that this would remove the need to send the patch to the list anyway (or tell what the fix was if trivial). This doesn't either remove the rule that 'make check syntax-check' should pass before commiting anything, and obviously the existing review process is still needed t for anything which is not a trivial fix breaking make or checks. I guess it makes sense to minimize disruption for those working on head and lower the time needed to get those fix in for those who catch and fix them ;-) Opinions ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Thu, Oct 16, 2008 at 03:15:08PM +0200, Daniel Veillard wrote:
As the number of compilation options and platform grows, it gets more difficult for a commiter to always ensure one chunk of code won't give a problem in a different situation. To try to lower the cost of maintaining the protability I would suggest the following rule for commit: - if a recently commited patch breaks compilation on a platform or for a given driver then it's fine to commit a minimal fix directly without getting the review feedback first - similary if make check or make syntax-chek breaks, if there is an obvious fix, it's fine to commit immediately Note that this would remove the need to send the patch to the list anyway (or tell what the fix was if trivial). This doesn't either remove the rule that 'make check syntax-check' should pass before commiting anything, and obviously the existing review process is still needed t for anything which is not a trivial fix breaking make or checks.
I guess it makes sense to minimize disruption for those working on head and lower the time needed to get those fix in for those who catch and fix them ;-) Opinions ?
Yes, this is reasonable idea - its crazy to wait for ACKs on the list to fix 1-2 line typos in the code which break compilation. I'll also remind all those with commit access that you should run be running autogen.sh or configure with --enable-compile-warnings=error which adds -Werror to compile flags, so no warnings get missed. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel Berrange <berrange@redhat.com> wrote:
On Thu, Oct 16, 2008 at 03:15:08PM +0200, Daniel Veillard wrote: ...
I guess it makes sense to minimize disruption for those working on head and lower the time needed to get those fix in for those who catch and fix them ;-) Opinions ?
Yes, this is reasonable idea - its crazy to wait for ACKs on the list to fix 1-2 line typos in the code which break compilation.
I think so too, and have just committed this: build: exempt *.ico files from the trailing blank check * .x-sc_trailing_blank: Add \.ico$ to the list.
On Thu, Oct 16, 2008 at 03:15:08PM +0200, Daniel Veillard wrote:
As the number of compilation options and platform grows, it gets more difficult for a commiter to always ensure one chunk of code won't give a problem in a different situation. To try to lower the cost of maintaining the protability I would suggest the following rule for commit: - if a recently commited patch breaks compilation on a platform or for a given driver then it's fine to commit a minimal fix directly without getting the review feedback first - similary if make check or make syntax-chek breaks, if there is an obvious fix, it's fine to commit immediately Note that this would remove the need to send the patch to the list
would *not* .... sigh !
anyway (or tell what the fix was if trivial). This doesn't either remove the rule that 'make check syntax-check' should pass before
Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
Hi, Daniel This proposal seems approved. Does this proposal message goes to somewhere in libvirt document? Thanks Atsushi SAKAI Daniel Veillard <veillard@redhat.com> wrote:
As the number of compilation options and platform grows, it gets more difficult for a commiter to always ensure one chunk of code won't give a problem in a different situation. To try to lower the cost of maintaining the protability I would suggest the following rule for commit: - if a recently commited patch breaks compilation on a platform or for a given driver then it's fine to commit a minimal fix directly without getting the review feedback first - similary if make check or make syntax-chek breaks, if there is an obvious fix, it's fine to commit immediately Note that this would remove the need to send the patch to the list anyway (or tell what the fix was if trivial). This doesn't either remove the rule that 'make check syntax-check' should pass before commiting anything, and obviously the existing review process is still needed t for anything which is not a trivial fix breaking make or checks.
I guess it makes sense to minimize disruption for those working on head and lower the time needed to get those fix in for those who catch and fix them ;-) Opinions ?
Daniel
-- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Oct 16, 2008 at 10:41:02PM +0900, Atsushi SAKAI wrote:
Hi, Daniel
This proposal seems approved. Does this proposal message goes to somewhere in libvirt document?
Well typos fixes for documentation and code comments could be managed in the same way. It doesn't break builds, and very few people ;-) seems to chase them. As long as the patch is still sent to the list, that would be fine by me ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Thu, Oct 16, 2008 at 10:41:02PM +0900, Atsushi SAKAI wrote:
Hi, Daniel
This proposal seems approved. Does this proposal message goes to somewhere in libvirt document?
Jim pointed out I completely misunderstood your comment :-) Yes you're right, it should be generally documented. I suggest adding the following new section at the end of the HACKING file to document the commiting policies: -------------------------------- Libvirt commiters guidelines ============================ The AUTHORS files indicates the list of people with commit acces right who can actually merge the patches. The general rule for commiting patches is to make sure it has been reviewed properly in the mailing-list first, usually if a couple of persons gave an ACK or +1 to a patch and nobody raised an objection on the list it should be good to go. If the patch touches a part of the code where you're not the main maintainer or not have a very clear idea of how things work, it's better to wait for a more authoritative feedback though. Before commiting please also rebuild locally and run 'make check syntax-check' and make sure they don't raise error. Try to look for warnings too for example configure with --enable-compile-warnings=error which adds -Werror to compile flags, so no warnings get missed Exceptions to that 'review and approval on the list first' is fixing failures to build: - if a recently commited patch breaks compilation on a platform or for a given driver then it's fine to commit a minimal fix directly without getting the review feedback first - similary if make check or make syntax-chek breaks, if there is an obvious fix, it's fine to commit immediately The patch should still be sent to the list (or tell what the fix was if trivial) and 'make check syntax-check' should pass too before commiting anything Similary typo fixes for documentation and code comments can be managed in the same way, but still make sure they get reviewed if non-trivial. -------------------------------- I think it documents the current practice, feedback welcome :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (4)
-
Atsushi SAKAI -
Daniel Berrange -
Daniel Veillard -
Jim Meyering