Re: [ovirt-devel] Gerrit headers are not added to commits in vdsm repo

On Fri, Nov 25, 2016 at 4:45 PM, Tomáš Golembiovský <tgolembi@redhat.com> wrote:
Hi,
I've noticed that in vdsm repo the merged commits do not contain the info headers added by Gerrit any more (Reviewed-by/Reviewed-on/etc.).
Is that intentional? If yes, what was the motivation behind this?
The change seem to have happened about 4 days ago. Sometime between the following two commits:
* 505f5da API: Introduce getQemuImageInfo API. [Maor Lipchuk] * 1c4a39c protocoldetector: Avoid unneeded getpeername() [Nir Soffer]
We switched vdsm to fast-forward 4 days ago, maybe this was unintended side effect of this change? The gerrit headers are very useful, please add back. Nir

Not sure I understand what do you mean by Gerrit Headers. Can you give examples? On Fri, Nov 25, 2016 at 4:57 PM, Nir Soffer <nsoffer@redhat.com> wrote:
On Fri, Nov 25, 2016 at 4:45 PM, Tomáš Golembiovský <tgolembi@redhat.com> wrote:
Hi,
I've noticed that in vdsm repo the merged commits do not contain the info headers added by Gerrit any more (Reviewed-by/Reviewed-on/etc.).
Is that intentional? If yes, what was the motivation behind this?
The change seem to have happened about 4 days ago. Sometime between the following two commits:
* 505f5da API: Introduce getQemuImageInfo API. [Maor Lipchuk] * 1c4a39c protocoldetector: Avoid unneeded getpeername() [Nir Soffer]
We switched vdsm to fast-forward 4 days ago, maybe this was unintended side effect of this change?
The gerrit headers are very useful, please add back.
Nir _______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
-- Eyal Edri Associate Manager RHV DevOps EMEA ENG Virtualization R&D Red Hat Israel phone: +972-9-7692018 irc: eedri (on #tlv #rhev-dev #rhev-integ)

On Sun, Nov 27, 2016 at 12:31 PM, Eyal Edri <eedri@redhat.com> wrote:
Not sure I understand what do you mean by Gerrit Headers. Can you give examples?
Here the point when the header disappeared: commit 82bebee084cb9841a42cc9131a724362771a2c80 Author: Petr Horáček <phoracek@redhat.com> Date: Wed Nov 16 15:28:01 2016 +0100 net: enable link.bond to handle options Note that this change also requires change in slaves editation (in _revert_transaction) since we cannot edit options of a bonding with attached slaves. Change-Id: I4399432347dc00f11d6bea28d731dd6e37914c20 Signed-off-by: Petr Horáček <phoracek@redhat.com> Bug-Url: https://bugzilla.redhat.com/1379115 commit 505f5da01785ece0e238a7fa3125358b67595d69 Author: Maor Lipchuk <mlipchuk@redhat.com> Date: Wed Nov 9 16:01:04 2016 +0200 API: Introduce getQemuImageInfo API. The new API suppose to return information fetched using qemuimg info. This should be usable to get the compat version of the volume. This API will not use a job since it only returns information about the volume and do not change it. It is similar to how getVolumeInfo is working. Change-Id: Ic170e2f142e8e70b534083c9468d249fe2478943 Signed-off-by: Maor Lipchuk <mlipchuk@redhat.com> Reviewed-on: https://gerrit.ovirt.org/66295 Reviewed-by: Nir Soffer <nsoffer@redhat.com> Continuous-Integration: Jenkins CI Note that the older commit had: - Reviewed-on: - Reviewed-by: - Continuous-Integration:
On Fri, Nov 25, 2016 at 4:57 PM, Nir Soffer <nsoffer@redhat.com> wrote:
On Fri, Nov 25, 2016 at 4:45 PM, Tomáš Golembiovský <tgolembi@redhat.com> wrote:
Hi,
I've noticed that in vdsm repo the merged commits do not contain the info headers added by Gerrit any more (Reviewed-by/Reviewed-on/etc.).
Is that intentional? If yes, what was the motivation behind this?
The change seem to have happened about 4 days ago. Sometime between the following two commits:
* 505f5da API: Introduce getQemuImageInfo API. [Maor Lipchuk] * 1c4a39c protocoldetector: Avoid unneeded getpeername() [Nir Soffer]
We switched vdsm to fast-forward 4 days ago, maybe this was unintended side effect of this change?
The gerrit headers are very useful, please add back.
Nir _______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra
-- Eyal Edri Associate Manager RHV DevOps EMEA ENG Virtualization R&D Red Hat Israel
phone: +972-9-7692018 irc: eedri (on #tlv #rhev-dev #rhev-integ)

On Sun, Nov 27, 2016 at 12:31:21PM +0200, Eyal Edri wrote:
Not sure I understand what do you mean by Gerrit Headers. Can you give examples?
On Fri, Nov 25, 2016 at 4:57 PM, Nir Soffer <nsoffer@redhat.com> wrote:
On Fri, Nov 25, 2016 at 4:45 PM, Tomáš Golembiovský <tgolembi@redhat.com> wrote:
Hi,
I've noticed that in vdsm repo the merged commits do not contain the info headers added by Gerrit any more (Reviewed-by/Reviewed-on/etc.).
Is that intentional? If yes, what was the motivation behind this?
The change seem to have happened about 4 days ago. Sometime between the following two commits:
* 505f5da API: Introduce getQemuImageInfo API. [Maor Lipchuk] * 1c4a39c protocoldetector: Avoid unneeded getpeername() [Nir Soffer]
We switched vdsm to fast-forward 4 days ago, maybe this was unintended side effect of this change?
The gerrit headers are very useful, please add back.
https://gerrit.ovirt.org/#/c/66295/ is the last one which had them: Reviewed-on: https://gerrit.ovirt.org/66295 Reviewed-by: Nir Soffer <nsoffer@redhat.com> Continuous-Integration: Jenkins CI they are added to the commit message during cherry-pick, and I find them very useful.

I don't see any options to control this from project config, it will require more investigating to see if its a config option or only available via cherry-pick. opening a ticket to track this. On Sun, Nov 27, 2016 at 1:38 PM, Dan Kenigsberg <danken@redhat.com> wrote:
Not sure I understand what do you mean by Gerrit Headers. Can you give examples?
On Fri, Nov 25, 2016 at 4:57 PM, Nir Soffer <nsoffer@redhat.com> wrote:
On Fri, Nov 25, 2016 at 4:45 PM, Tomáš Golembiovský < tgolembi@redhat.com> wrote:
Hi,
I've noticed that in vdsm repo the merged commits do not contain the info headers added by Gerrit any more (Reviewed-by/Reviewed-on/etc.) .
Is that intentional? If yes, what was the motivation behind this?
The change seem to have happened about 4 days ago. Sometime between
On Sun, Nov 27, 2016 at 12:31:21PM +0200, Eyal Edri wrote: the
following two commits:
* 505f5da API: Introduce getQemuImageInfo API. [Maor Lipchuk] * 1c4a39c protocoldetector: Avoid unneeded getpeername() [Nir Soffer]
We switched vdsm to fast-forward 4 days ago, maybe this was unintended side effect of this change?
The gerrit headers are very useful, please add back.
https://gerrit.ovirt.org/#/c/66295/ is the last one which had them:
Reviewed-on: https://gerrit.ovirt.org/66295 Reviewed-by: Nir Soffer <nsoffer@redhat.com> Continuous-Integration: Jenkins CI
they are added to the commit message during cherry-pick, and I find them very useful.
-- Eyal Edri Associate Manager RHV DevOps EMEA ENG Virtualization R&D Red Hat Israel phone: +972-9-7692018 irc: eedri (on #tlv #rhev-dev #rhev-integ)

On 25 November 2016 at 16:57, Nir Soffer <nsoffer@redhat.com> wrote:
On Fri, Nov 25, 2016 at 4:45 PM, Tomáš Golembiovský <tgolembi@redhat.com> wrote:
Hi,
I've noticed that in vdsm repo the merged commits do not contain the info headers added by Gerrit any more (Reviewed-by/Reviewed-on/etc.).
Is that intentional? If yes, what was the motivation behind this?
The change seem to have happened about 4 days ago. Sometime between the following two commits:
* 505f5da API: Introduce getQemuImageInfo API. [Maor Lipchuk] * 1c4a39c protocoldetector: Avoid unneeded getpeername() [Nir Soffer]
We switched vdsm to fast-forward 4 days ago, maybe this was unintended side effect of this change?
The gerrit headers are very useful, please add back.
Headers cannot be added in fast-forward mode b/c then you end up with a new commit hash - not fast forwarding to an existing commit. In other words - headers are only added when Gerrit creates a new commit - which in FF mode it never does. I could move the project to "Rebase Always" which is like "Rebase if Necessary" but always creates a new commit (with headers). Please note that this is less strict then ff-only and therefore would lead to merged combinations that do not get tested in CI. -- Barak Korren bkorren@redhat.com RHEV-CI Team

On Sun, Nov 27, 2016 at 4:34 PM, Barak Korren <bkorren@redhat.com> wrote:
On 25 November 2016 at 16:57, Nir Soffer <nsoffer@redhat.com> wrote:
On Fri, Nov 25, 2016 at 4:45 PM, Tomáš Golembiovský <tgolembi@redhat.com> wrote:
Hi,
I've noticed that in vdsm repo the merged commits do not contain the info headers added by Gerrit any more (Reviewed-by/Reviewed-on/etc.).
Is that intentional? If yes, what was the motivation behind this?
The change seem to have happened about 4 days ago. Sometime between the following two commits:
* 505f5da API: Introduce getQemuImageInfo API. [Maor Lipchuk] * 1c4a39c protocoldetector: Avoid unneeded getpeername() [Nir Soffer]
We switched vdsm to fast-forward 4 days ago, maybe this was unintended side effect of this change?
The gerrit headers are very useful, please add back.
Headers cannot be added in fast-forward mode b/c then you end up with a new commit hash - not fast forwarding to an existing commit.
In other words - headers are only added when Gerrit creates a new commit - which in FF mode it never does.
Makes sense.
I could move the project to "Rebase Always" which is like "Rebase if Necessary" but always creates a new commit (with headers). Please note that this is less strict then ff-only and therefore would lead to merged combinations that do not get tested in CI.
Or we can add this url before posting a patch (using commit hook): https://gerrit.ovirt.org/#/q/change:I71a2ae262482edc8acf303f18eac6f9035e710b... Also works for truncated change id: https://gerrit.ovirt.org/#/q/change:I71a2ae262482edc This will show also the backports for a change, even more useful. Nir
participants (4)
-
Barak Korren
-
Dan Kenigsberg
-
Eyal Edri
-
Nir Soffer