Stable branch gerrit hook and Related-To: bug
Hi, I would like to ask if it might be possible for the gerrit hook to stop giving -1 to patches that do not have bug-url, but have related-to. The reason for this is that I have a rather large patchset that is adding a new feature. No single patch by itself fixes the referenced bug and there are also patches that are not related to the feature itself, but were backported because they did some refactoring that was needed for the code to apply and/or work properly. -- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
Hi, =20 I would like to ask if it might be possible for the gerrit hook to stop g= iving -1 to patches that do not have bug-url, but have related-to. =20 The reason for this is that I have a rather large patchset that is adding= a new feature. No single patch by itself fixes the referenced bug and ther= e are also patches that are not related to the feature itself, but were bac= kported because they did some refactoring that was needed for the code to a=
--fCcDWlUEdh43YKr8 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 08/27, Martin Sivak wrote: pply and/or work properly. I have a couple question, which of these should happen for the Related-To bugs? * Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no * Should it update the external trackers of the bug? Most probably yes
=20 -- Martin Siv=E1k msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ =20 _______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
--=20 David Caro Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605 --fCcDWlUEdh43YKr8 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJT/xRpAAoJEEBxx+HSYmnDYOwH/i1tjpLYgdbDxXMbrCubTxNt TuIyH/C962vZ3478Ukqh30tarXTIc0wn+d0Qi3QNA4UfTgE0+UHSlUUn+VS8UTFd +cfkNNLSLnRRwyESs6/CCF7+ExOBhvPKiKl1l7HVknp8Lt/4xtFbZY4HLDG61DJE bWyXptI/4gOAFYuO48G07WcVAG/6uMVabq6ynx2xfij04G+lMw+52pS28cJNcxgP HZvC/i63wjlGDQ3ZvchzNaWl+8bXS9bT74CwHgeHU+VKcysd4mfdgsPtl8il0uq2 i/o43GsAY6hPzAP1KMTjk0vgMQZZ02Sak+mELj8isgS3TE7HhEHbiKslUbHI+TU= =8bpq -----END PGP SIGNATURE----- --fCcDWlUEdh43YKr8--
Hi, yeah I think it should be like that.
* Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no * Should it update the external trackers of the bug? Most probably yes
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ ----- Original Message -----
On 08/27, Martin Sivak wrote:
Hi,
I would like to ask if it might be possible for the gerrit hook to stop giving -1 to patches that do not have bug-url, but have related-to.
The reason for this is that I have a rather large patchset that is adding a new feature. No single patch by itself fixes the referenced bug and there are also patches that are not related to the feature itself, but were backported because they did some refactoring that was needed for the code to apply and/or work properly.
I have a couple question, which of these should happen for the Related-To bugs?
* Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no * Should it update the external trackers of the bug? Most probably yes
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
_______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
-- David Caro
Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D
Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605
On Aug 28, 2014, at 16:14 , Martin Sivak <msivak@redhat.com> wrote:
Hi,
yeah I think it should be like that.
+1
* Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no * Should it update the external trackers of the bug? Most probably yes
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
----- Original Message -----
On 08/27, Martin Sivak wrote:
Hi,
I would like to ask if it might be possible for the gerrit hook to stop giving -1 to patches that do not have bug-url, but have related-to.
The reason for this is that I have a rather large patchset that is adding a new feature. No single patch by itself fixes the referenced bug and there are also patches that are not related to the feature itself, but were backported because they did some refactoring that was needed for the code to apply and/or work properly.
I have a couple question, which of these should happen for the Related-To bugs?
* Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no * Should it update the external trackers of the bug? Most probably yes
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
_______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
-- David Caro
Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D
Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605
_______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
On Thursday, August 28, 2014 01:37:13 PM David Caro wrote:
On 08/27, Martin Sivak wrote:
Hi,
I would like to ask if it might be possible for the gerrit hook to stop giving -1 to patches that do not have bug-url, but have related-to.
The reason for this is that I have a rather large patchset that is adding a new feature. No single patch by itself fixes the referenced bug and there are also patches that are not related to the feature itself, but were backported because they did some refactoring that was needed for the code to apply and/or work properly. I have a couple question, which of these should happen for the Related-To bugs?
* Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no
About this, the patches that have the Related-To should be taken into account when changing the bug to MODIFIED? If so the bug will not be changed to MODIFIED if the last open bug uses Related-To tab instead og Bug-Url. If not, you might end up with open patches added t a bug that is on MODIFIED state. And you will have bugs not on POST state that have external bugs, as those bugs will not change the status. Is that ok? I can add a message to one of the fields of the bug so it's easy to see it's a related bug.
* Should it update the external trackers of the bug? Most probably yes
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
_______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
-- David Caro Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Martin Sivak" <msivak@redhat.com> Cc: devel@ovirt.org Sent: Tuesday, September 16, 2014 2:59:03 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
On Thursday, August 28, 2014 01:37:13 PM David Caro wrote:
On 08/27, Martin Sivak wrote:
Hi,
I would like to ask if it might be possible for the gerrit hook to stop giving -1 to patches that do not have bug-url, but have related-to.
The reason for this is that I have a rather large patchset that is adding a new feature. No single patch by itself fixes the referenced bug and there are also patches that are not related to the feature itself, but were backported because they did some refactoring that was needed for the code to apply and/or work properly. I have a couple question, which of these should happen for the Related-To bugs?
* Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no
About this, the patches that have the Related-To should be taken into account when changing the bug to MODIFIED?
If so the bug will not be changed to MODIFIED if the last open bug uses Related-To tab instead og Bug-Url.
Sounds good to me. Related-to shouldn't change a bug from post to modified.
If not, you might end up with open patches added t a bug that is on MODIFIED state.
And you will have bugs not on POST state that have external bugs, as those bugs will not change the status.
Is that ok? I can add a message to one of the fields of the bug so it's easy to see it's a related bug.
+1
* Should it update the external trackers of the bug? Most probably yes
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
_______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
-- David Caro
Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D
Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605 _______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Martin Sivak" <msivak@redhat.com> Cc: devel@ovirt.org Sent: Tuesday, September 16, 2014 2:59:03 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
On Thursday, August 28, 2014 01:37:13 PM David Caro wrote:
On 08/27, Martin Sivak wrote:
Hi,
I would like to ask if it might be possible for the gerrit hook to stop giving -1 to patches that do not have bug-url, but have related-to.
The reason for this is that I have a rather large patchset that is adding a new feature. No single patch by itself fixes the referenced bug and there are also patches that are not related to the feature itself, but were backported because they did some refactoring that was needed for the code to apply and/or work properly.
Why can't you put bug url on all of these patches? Relates-To: is just for reference.
I have a couple question, which of these should happen for the Related-To bugs?
* Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no
About this, the patches that have the Related-To should be taken into account when changing the bug to MODIFIED?
NO!
If so the bug will not be changed to MODIFIED if the last open bug uses Related-To tab instead og Bug-Url.
Right Nir
Why can't you put bug url on all of these patches?
Because Bug-Url does
About this, the patches that have the Related-To should be taken into account when changing the bug to MODIFIED?
NO!
this. -- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ ----- Original Message -----
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Martin Sivak" <msivak@redhat.com> Cc: devel@ovirt.org Sent: Tuesday, September 16, 2014 2:59:03 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
On Thursday, August 28, 2014 01:37:13 PM David Caro wrote:
On 08/27, Martin Sivak wrote:
Hi,
I would like to ask if it might be possible for the gerrit hook to stop giving -1 to patches that do not have bug-url, but have related-to.
The reason for this is that I have a rather large patchset that is adding a new feature. No single patch by itself fixes the referenced bug and there are also patches that are not related to the feature itself, but were backported because they did some refactoring that was needed for the code to apply and/or work properly.
Why can't you put bug url on all of these patches?
Relates-To: is just for reference.
I have a couple question, which of these should happen for the Related-To bugs?
* Should it pass the same checks than a Bug-Url? Most probably yes Right now that is: - Public - Target release - Product * Should it change the bug statuses too? Most probably no
About this, the patches that have the Related-To should be taken into account when changing the bug to MODIFIED?
NO!
If so the bug will not be changed to MODIFIED if the last open bug uses Related-To tab instead og Bug-Url.
Right
Nir
----- Original Message -----
From: "Martin Sivak" <msivak@redhat.com> To: "Nir Soffer" <nsoffer@redhat.com> Cc: "David Caro" <dcaroest@redhat.com>, devel@ovirt.org Sent: Tuesday, September 16, 2014 3:22:40 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
Why can't you put bug url on all of these patches?
Because Bug-Url does
About this, the patches that have the Related-To should be taken into account when changing the bug to MODIFIED?
NO!
this.
But it does this only when merged - you just have to state clearly that a series of patches must be merged together. If the maintainer ignores your note, it not your problem ;-) What if gerrit used the dependency info and would not -1 patches that are dependency of a patch with a bug url? Nir
What if gerrit used the dependency info and would not -1 patches that are dependency of a patch with a bug url?
That is not a bad idea, but there is one use case you do not consider.. it would mean that feature cannot be merged before it is completed. I like my patches to be merged as soon as possible. Of course if the patches leave the tree in working state. The patchset that started this thread already depends on about 15 patches that were merged to master two moths ago. -- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ ----- Original Message -----
----- Original Message -----
From: "Martin Sivak" <msivak@redhat.com> To: "Nir Soffer" <nsoffer@redhat.com> Cc: "David Caro" <dcaroest@redhat.com>, devel@ovirt.org Sent: Tuesday, September 16, 2014 3:22:40 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
Why can't you put bug url on all of these patches?
Because Bug-Url does
About this, the patches that have the Related-To should be taken into account when changing the bug to MODIFIED?
NO!
this.
But it does this only when merged - you just have to state clearly that a series of patches must be merged together. If the maintainer ignores your note, it not your problem ;-)
What if gerrit used the dependency info and would not -1 patches that are dependency of a patch with a bug url?
Nir
----- Original Message -----
From: "Martin Sivak" <msivak@redhat.com> To: "Nir Soffer" <nsoffer@redhat.com> Cc: "David Caro" <dcaroest@redhat.com>, devel@ovirt.org Sent: Tuesday, September 16, 2014 3:53:53 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
What if gerrit used the dependency info and would not -1 patches that are dependency of a patch with a bug url?
That is not a bad idea, but there is one use case you do not consider.. it would mean that feature cannot be merged before it is completed.
I like my patches to be merged as soon as possible. Of course if the patches leave the tree in working state.
Right
The patchset that started this thread already depends on about 15 patches that were merged to master two moths ago.
So how about: Required-For: https://bugzilla.redhat.com/1096197 You must merge this to get the bug fixed, but the bug status is not changed when merged. The hook can add the patch to the bug as a tracker. David, can we easily add such tag? NIr
Well Related-To means exactly what you are proposing where I came from (the platform group, distgit, errata..) :) But whatever suits you. Martin -- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ ----- Original Message -----
----- Original Message -----
From: "Martin Sivak" <msivak@redhat.com> To: "Nir Soffer" <nsoffer@redhat.com> Cc: "David Caro" <dcaroest@redhat.com>, devel@ovirt.org Sent: Tuesday, September 16, 2014 3:53:53 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
What if gerrit used the dependency info and would not -1 patches that are dependency of a patch with a bug url?
That is not a bad idea, but there is one use case you do not consider.. it would mean that feature cannot be merged before it is completed.
I like my patches to be merged as soon as possible. Of course if the patches leave the tree in working state.
Right
The patchset that started this thread already depends on about 15 patches that were merged to master two moths ago.
So how about:
Required-For: https://bugzilla.redhat.com/1096197
You must merge this to get the bug fixed, but the bug status is not changed when merged.
The hook can add the patch to the bug as a tracker.
David, can we easily add such tag?
NIr
On Tuesday, September 16, 2014 11:22:11 AM Martin Sivak wrote:
Well Related-To means exactly what you are proposing where I came from (the platform group, distgit, errata..) :)
But whatever suits you.
Martin
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
----- Original Message -----
----- Original Message -----
From: "Martin Sivak" <msivak@redhat.com> To: "Nir Soffer" <nsoffer@redhat.com> Cc: "David Caro" <dcaroest@redhat.com>, devel@ovirt.org Sent: Tuesday, September 16, 2014 3:53:53 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
What if gerrit used the dependency info and would not -1 patches that are dependency of a patch with a bug url?
That is not a bad idea, but there is one use case you do not consider.. it would mean that feature cannot be merged before it is completed.
I like my patches to be merged as soon as possible. Of course if the patches leave the tree in working state.
Right
The patchset that started this thread already depends on about 15 patches that were merged to master two moths ago.
So how about:
Required-For: https://bugzilla.redhat.com/1096197
You must merge this to get the bug fixed, but the bug status is not changed when merged.
The hook can add the patch to the bug as a tracker.
David, can we easily add such tag?
No problem, but can you please summarize what needs to be done for each? If you open a track ticket with that info even better :) - When patch created (move bug to POST, add external tracker, other....) - When patch merged (move bug to post if not more open external patches) - Same checks that with Bug-Url (good product, target release for stable branch, public bug, ...) * About the Related-To tag * About the Required-For tag Thanks!
NIr
-- David Caro Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605
----- Original Message -----
From: "David Caro" <dcaroest@redhat.com> To: "Martin Sivak" <msivak@redhat.com> Cc: devel@ovirt.org Sent: Tuesday, September 16, 2014 7:10:48 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
On Tuesday, September 16, 2014 11:22:11 AM Martin Sivak wrote:
Well Related-To means exactly what you are proposing where I came from (the platform group, distgit, errata..) :)
But whatever suits you.
Martin
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
----- Original Message -----
----- Original Message -----
From: "Martin Sivak" <msivak@redhat.com> To: "Nir Soffer" <nsoffer@redhat.com> Cc: "David Caro" <dcaroest@redhat.com>, devel@ovirt.org Sent: Tuesday, September 16, 2014 3:53:53 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
What if gerrit used the dependency info and would not -1 patches that are dependency of a patch with a bug url?
That is not a bad idea, but there is one use case you do not consider.. it would mean that feature cannot be merged before it is completed.
I like my patches to be merged as soon as possible. Of course if the patches leave the tree in working state.
Right
The patchset that started this thread already depends on about 15 patches that were merged to master two moths ago.
So how about:
Required-For: https://bugzilla.redhat.com/1096197
You must merge this to get the bug fixed, but the bug status is not changed when merged.
The hook can add the patch to the bug as a tracker.
David, can we easily add such tag? stuff that's
No problem, but can you please summarize what needs to be done for each? If you open a track ticket with that info even better :)
- When patch created (move bug to POST, add external tracker, other....) - When patch merged (move bug to post if not more open external patches) - Same checks that with Bug-Url (good product, target release for stable branch, public bug, ...)
* About the Related-To tag * About the Required-For tag
The Related-To is enough, there's no need for such over-head, it will just create unnecessary extra work for David/Marin and confusion between the specific tag usage. Let's keep it simple. Sometimes we just write patches that don't fix the bug, but we need them merged in order to write more patches that actually fix the bug. We just need to define what we need them to do, which is - 1. Adding the patches as a tracker to the bz is a nice-to-have. 2. We must be able to merge them into the stable branch. 3. They may change the bz to post but never to modified. Let's not forget that in special cases we can always manually change the bz state. The common case for the Related-To patches can be handled as mentioned above. Vered
Thanks!
NIr
-- David Caro
Red Hat S.L. Continuous Integration Engineer - EMEA ENG Virtualization R&D
Tel.: +420 532 294 605 Email: dcaro@redhat.com Web: www.redhat.com RHT Global #: 82-62605 _______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
Why should an incomplete, thus non working feature get merged? Do you also merge incomplete bugfixes? One commit should contain one feature or one bugfix, imho? I know this can be difficult sometimes, but still, at least in the master branch this rule should get enforced, maybe create a staging branch for incomplete features? On 16/09/14 14:53, Martin Sivak wrote:
it would mean that feature cannot be merged before it is completed.
-- Mit freundlichen Grüßen / Regards Sven Kieske Systemadministrator Mittwald CM Service GmbH & Co. KG Königsberger Straße 6 32339 Espelkamp T: +49-5772-293-100 F: +49-5772-293-333 https://www.mittwald.de Geschäftsführer: Robert Meyer St.Nr.: 331/5721/1033, USt-IdNr.: DE814773217, HRA 6640, AG Bad Oeynhausen Komplementärin: Robert Meyer Verwaltungs GmbH, HRB 13260, AG Bad Oeynhausen
----- Original Message -----
From: "Sven Kieske" <s.kieske@mittwald.de> To: devel@ovirt.org Sent: Wednesday, September 17, 2014 12:01:50 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
Why should an incomplete, thus non working feature get merged? Do you also merge incomplete bugfixes?
One commit should contain one feature or one bugfix, imho? I know this can be difficult sometimes, but still, at least in the master branch this rule should get enforced, maybe create a staging branch for incomplete features?
On 16/09/14 14:53, Martin Sivak wrote:
it would mean that feature cannot be merged before it is completed.
If you look again at Martin's mail, he wrote:
That is not a bad idea, but there is one use case you do not consider.. it would mean that feature cannot be merged before it is completed. I like my patches to be merged as soon as possible. Of course if the patches leave the tree in working state.
And I agree with him. There is no harm in merging to master branch changes that do not harm it, even make it better, just because they do not completely fix an existing bugzilla bug. Actually I think the opposite is true - a large bug that requires a large and complex change is usually better split to smaller changes that are easier to review/verify/etc. -- Didi
----- Original Message -----
From: "Yedidyah Bar David" <didi@redhat.com> To: "Sven Kieske" <s.kieske@mittwald.de> Cc: devel@ovirt.org Sent: Wednesday, September 17, 2014 11:08:52 AM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
That is not a bad idea, but there is one use case you do not consider.. it would mean that feature cannot be merged before it is completed. I like my patches to be merged as soon as possible. Of course if the patches leave the tree in working state.
And I agree with him. There is no harm in merging to master branch changes that do not harm it, even make it better, just because they do not completely fix an existing bugzilla bug. Actually I think the opposite is true - a large bug that requires a large and complex change is usually better split to smaller changes that are easier to review/verify/etc.
+1 In some cases may be feasible -or actually better!- to pack the change in a single patch, but in general I believe it is better in every way if a change has its dependencies - maybe refactoring, maybe adding helper code and its tests split in different patches; this also makes review and backporting easier. Of course, the prerequisite is that the branch must be usable and stable after each change, but we always were in complete agreement about that. Bests, -- Francesco Romani RedHat Engineering Virtualization R & D Phone: 8261328 IRC: fromani
----- Original Message -----
From: "Sven Kieske" <s.kieske@mittwald.de> To: devel@ovirt.org Sent: Wednesday, September 17, 2014 12:01:50 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
Why should an incomplete, thus non working feature get merged?
Adding a feature may require improving the design of the existing code. This may require multiple otherwise unrelated changes. The best way to add these changes is to make small clear and safe changes that leave the code in functional state. Reviewing and verifying small changes is much easier and safer and results in less bugs and more stable code.
Do you also merge incomplete bugfixes?
There is no such thing as "complete" in software or in life :-) In general, bug fixes should be small and simple as possible, to limit the risk to a stable code. Such fixes may be sometimes incomplete, since a complete fix would required a risky redesign. Nir
Hi,
Why should an incomplete, thus non working feature get merged? Do you also merge incomplete bugfixes? One commit should contain one feature or one bugfix, imho?
Let me rephrase your sentence a bit: One commit should contain one self-contained and isolated change. But that does not mean the change is enough to fix the referenced bug. Example: Imagine a situation where you need to add new API to networking part, extend the API in the storage area and then add something new that uses it. Doing this all in a single patch is hard because: 1) All reviewers will have to go through all the code 2) If the development takes time you have to solve conflicts in all the areas all the time On the other hand if you split that into three patches: - Network API improvement (you can go even further and add it call by call where each patch contains new call and relevant test) - Storage API change - New feature Then it is much easier to review. And you can merge the API changes as soon as they are ready and avoid future merge conflicts. Gerrit actually supports this workflow in the form of topic branches. -- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ ----- Original Message -----
Why should an incomplete, thus non working feature get merged? Do you also merge incomplete bugfixes?
One commit should contain one feature or one bugfix, imho? I know this can be difficult sometimes, but still, at least in the master branch this rule should get enforced, maybe create a staging branch for incomplete features?
On 16/09/14 14:53, Martin Sivak wrote:
it would mean that feature cannot be merged before it is completed.
-- Mit freundlichen Grüßen / Regards
Sven Kieske
Systemadministrator Mittwald CM Service GmbH & Co. KG Königsberger Straße 6 32339 Espelkamp T: +49-5772-293-100 F: +49-5772-293-333 https://www.mittwald.de Geschäftsführer: Robert Meyer St.Nr.: 331/5721/1033, USt-IdNr.: DE814773217, HRA 6640, AG Bad Oeynhausen Komplementärin: Robert Meyer Verwaltungs GmbH, HRB 13260, AG Bad Oeynhausen _______________________________________________ Devel mailing list Devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/devel
----- Original Message -----
From: "Martin Sivak" <msivak@redhat.com> To: "Sven Kieske" <s.kieske@mittwald.de> Cc: devel@ovirt.org Sent: Wednesday, September 17, 2014 12:27:29 PM Subject: Re: [ovirt-devel] Stable branch gerrit hook and Related-To: bug
Hi,
Why should an incomplete, thus non working feature get merged? Do you also merge incomplete bugfixes? One commit should contain one feature or one bugfix, imho?
Let me rephrase your sentence a bit:
One commit should contain one self-contained and isolated change.
But that does not mean the change is enough to fix the referenced bug.
Example:
Imagine a situation where you need to add new API to networking part, extend the API in the storage area and then add something new that uses it. Doing this all in a single patch is hard because:
1) All reviewers will have to go through all the code 2) If the development takes time you have to solve conflicts in all the areas all the time
On the other hand if you split that into three patches:
- Network API improvement (you can go even further and add it call by call where each patch contains new call and relevant test) - Storage API change - New feature
Then it is much easier to review. And you can merge the API changes as soon as they are ready and avoid future merge conflicts. Gerrit actually supports this workflow in the form of topic branches.
-- Martin Sivák msivak@redhat.com Red Hat Czech RHEV-M SLA / Brno, CZ
----- Original Message -----
Why should an incomplete, thus non working feature get merged? Do you also merge incomplete bugfixes?
One commit should contain one feature or one bugfix, imho? I know this can be difficult sometimes, but still, at least in the master branch this rule should get enforced, maybe create a staging branch for incomplete features?
On 16/09/14 14:53, Martin Sivak wrote:
it would mean that feature cannot be merged before it is completed.
-- Mit freundlichen Grüßen / Regards
Sven Kieske
Systemadministrator Mittwald CM Service GmbH & Co. KG Königsberger Straße 6 32339 Espelkamp T: +49-5772-293-100 F: +49-5772-293-333 https://www.mittwald.de Geschäftsführer: Robert Meyer St.Nr.: 331/5721/1033, USt-IdNr.: DE814773217, HRA 6640, AG Bad Oeynhausen Komplementärin: Robert Meyer Verwaltungs GmbH, HRB 13260, AG Bad Oeynhausen
Somehow I feel this is being discussed to death. We sometimes have features which take more than a single version to develop and we need to merge some of the work as an infrastructure. In whichcase the above suggestion makes sense, as working out of tree for more than a version is almost impossible. So, if someone has any hard objection or a new argument please step forward. Otherwise I see no reason to deny this suggestion which will assist developers as long as they keep the above guidelines. Doron
participants (9)
-
David Caro -
Doron Fediuck -
Francesco Romani -
Martin Sivak -
Michal Skrivanek -
Nir Soffer -
Sven Kieske -
Vered Volansky -
Yedidyah Bar David