Bug 1428047

Marking this as NEEDINFO against myself, so that I can unblock it once feedback from the community is recieved over the devel list.

Also [1] in description is missing and is now posted here,

[1] http://lists.gluster.org/pipermail/gluster-devel/2017-March/052216.html

REVIEW: https://review.gluster.org/17004 (scripts: Update rfc.sh to check existance of Change-Id in backports) posted (#1) for review on master by Shyamsundar Ranganathan (srangana)

The initial change to enforce/remind contributors to this requirement is to update rfc.sh. The commit in comment #2 is towards the same.

Once that is approved, I will possibly move this bug to the infra queue, for a Jenkins job that can post a comment to non-master commits when the Change-Id is not found in master.

The latter is to serve as a checkpoint for reviewers so that this aspect is not missed.

REVIEW: https://review.gluster.org/17004 (scripts: Update rfc.sh to check existance of Change-Id in backports) posted (#2) for review on master by Shyamsundar Ranganathan (srangana)

REVIEW: https://review.gluster.org/17004 (scripts: Update rfc.sh to check existance of Change-Id in backports) posted (#3) for review on master by Shyamsundar Ranganathan (srangana)

COMMIT: https://review.gluster.org/17004 committed in master by Niels de Vos (ndevos) 
------
commit 788def7912c68616849748678574c60a52021e3c
Author: Shyam <srangana>
Date:   Wed Apr 5 14:22:57 2017 -0400

    scripts: Update rfc.sh to check existance of Change-Id in backports
    
    Addition to this script is a no-op on master.
    
    This would need to be backported to active release branches to be
    effective.
    
    This check is not smart proof, in that someone could proceed knowing
    that the Change-Id differs from master, but this is not expected to
    catch that, instead it is to serve more as a reminder that we need
    the same Change-Id across branches.
    
    Contributors not using rfc.sh would not see this, but they are few
    and possibly far in between. Also contributors using gerrit to
    cherry-pick changes will not see this. For both cases a server side
    solution to catch any changes are needed.
    
    There is a possiblilty that we will follow this up with a check
    on the gerrit end and add a comment to the reviews, to aid reviewers
    to quickly check the sanity of the Change-Id when it differs.
    
    Change-Id: I11e371489a4a3cf2ff96d9892256986cd535998b
    BUG: 1428047
    Signed-off-by: Shyam <srangana>
    Reviewed-on: https://review.gluster.org/17004
    Smoke: Gluster Build System <jenkins.org>
    Reviewed-by: Niels de Vos <ndevos>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Kaleb KEITHLEY <kkeithle>
    Reviewed-by: Amar Tumballi <amarts>

Moving this back to assigned, as we still need the Jenkins job to throw a warning on non-master review requests.

The blocks is incorrect, as this bug continues for the Jenkins (or other) server side checks and will get closed only when that is done, whereas the 3.10 and 3.8 bugs filed for this are just for the rfc.sh change. As a result the blocking relationship does not arise IMO

REVIEW: https://review.gluster.org/17418 (scripts: prevent a script warning if rfc.sh does not find a backport) posted (#1) for review on master by Niels de Vos (ndevos)

COMMIT: https://review.gluster.org/17418 committed in master by Shyamsundar Ranganathan (srangana) 
------
commit f618c7e606caaf8371aea4a02b352c987b3b1a40
Author: Niels de Vos <ndevos>
Date:   Tue May 30 13:10:50 2017 +0530

    scripts: prevent a script warning if rfc.sh does not find a backport
    
    When running ./rfc.sh on a branch to post release notes (not a backport
    from master), then the following script warning is displayed:
    
       ./rfc.sh: line 97: [: =: unary operator expected
    
    In case the used variables are not set, they are not handled as empty
    strings. Placing the variables inside "${qoutes}" prevents this warning.
    
    BUG: 1428047
    Change-Id: Ie171d6f66b47401d6ea4e78aa3ed2bd0c6fce9ce
    Signed-off-by: Niels de Vos <ndevos>
    Reviewed-on: https://review.gluster.org/17418
    Smoke: Gluster Build System <jenkins.org>
    NetBSD-regression: NetBSD Build System <jenkins.org>
    Reviewed-by: Kaleb KEITHLEY <kkeithle>
    CentOS-regression: Gluster Build System <jenkins.org>
    Reviewed-by: Shyamsundar Ranganathan <srangana>

This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.11.0, please open a new bug report.

glusterfs-3.11.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://lists.gluster.org/pipermail/announce/2017-May/000073.html
[2] https://www.gluster.org/pipermail/gluster-users/

Re-opening this, a Jenkins job to detect Change-ID anomolies would ease any discrepencies for reviewers and release-owners when merging changes, to understand and possibly educate on the need.

This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-3.12.0, please open a new bug report.

glusterfs-3.12.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://lists.gluster.org/pipermail/announce/2017-September/000082.html
[2] https://www.gluster.org/pipermail/gluster-users/

Keeps getting closed each time we make a release :)

Re-opening this, a Jenkins job to detect Change-ID anomolies would ease any discrepencies for reviewers and release-owners when merging changes, to understand and possibly educate on the need.

reopening as per comment #14

Can I have an example change and how our test should behave? I think we're ready to spend some time on this to get this moving.

Here is an easy example that is in my mind:

- The rfc.sh already does this job and let's the committer know that the change ID is not present in master (https://github.com/gluster/glusterfs/blob/master/rfc.sh#L70)

- The same check needs to be performed, and a message posted to the patch, that the Change-ID is either inconsistent or missing

- The job need not vote, as at times having different change IDs or not finding a change ID on master is a valid case (release notes, release specific patches are examples where the change ID in the branch will not be found in master)

The intent of the job is simply to remind reviewers that the change ID is different and we may want to cross check that to ensure that it is genuine and not an error by the committer.

Does the above help? Or, do you need more here?

I guess for now, the work we have done through ./rfc.sh is good. Prefer to close as WORKSFORME.