Hey guys,
See this note from Katie. Summary:
1) PCI compliance says that the code they deploy to the payments cluster must have peer-review. 2) There are 35 commits to core that are self-merged with no review (see Elliot's script).
Retroactively +1ing the changes is probably sufficient (after you've reviewed the change, of course :) ).
Deadline appears to be "before November".
Help?
Greg
PS: Katie is cc'd, but I don't think she is on ourlist.
---------- Forwarded message ---------- From: Katie Horn khorn@wikimedia.org Date: Tue, Oct 14, 2014 at 3:59 PM Subject: Self-reviews in core between 1.22 and 1.23 To: Greg Grossmeier greg@wikimedia.org
Hey Greg,
Thanks for looking in to this. Elliott came up with the following awk script and its subsequent output, both attached.
To summarize: PCI rules say that we can not deploy code to payments systems if there were any unreviewed (self-merged) patches, and we found about 35 in the upgrade of mediawiki core that fundraising would like to do before November.
It should be sufficient to have somebody other than the original committer retroactively +1 the patch in gerrit. Just so I can prove everything had extra eyes on it before we deploy the code to payments.
Please let me know if something went wrong with the attachments, and I will gladly resend.
Thanks! -Katie
---------- Forwarded message ---------- From: Elliott Eggleston eeggleston@wikimedia.org Date: Tue, Oct 14, 2014 at 3:48 PM Subject: Fwd: Fwd: For your inner PCI lawyer To: Katie Horn khorn@wikimedia.org
Do this log command and the attached awk script look correct?
git log --no-merges --show-notes=review --name-status fundraising/REL1_22..REL1_23 -- *php *php5 skins resources includes > allLog
./findSelfMerge.awk < allLog > unreviewed.txt
Hi,
I commented on a bunch of them with "+1" to mark them as "reviewed", all of them were either reverts, or trival doc changes of some kind.
Some of the ones in the list were properly reviewed, but jenkins failed them, so the original owner rebased and +2'd them. Do those need a re-review as well?
The ones on the top of the list were self-merged backports that were all properly reviewed on master, so I didn't +1 them.
-- Kunal
On 10/14/14 4:07 PM, Greg Grossmeier wrote:
Hey guys,
See this note from Katie. Summary:
- PCI compliance says that the code they deploy to the payments
cluster must have peer-review. 2) There are 35 commits to core that are self-merged with no review (see Elliot's script).
Retroactively +1ing the changes is probably sufficient (after you've reviewed the change, of course :) ).
Deadline appears to be "before November".
Help?
Greg
PS: Katie is cc'd, but I don't think she is on ourlist.
---------- Forwarded message ---------- From: Katie Horn khorn@wikimedia.org Date: Tue, Oct 14, 2014 at 3:59 PM Subject: Self-reviews in core between 1.22 and 1.23 To: Greg Grossmeier greg@wikimedia.org
Hey Greg,
Thanks for looking in to this. Elliott came up with the following awk script and its subsequent output, both attached.
To summarize: PCI rules say that we can not deploy code to payments systems if there were any unreviewed (self-merged) patches, and we found about 35 in the upgrade of mediawiki core that fundraising would like to do before November.
It should be sufficient to have somebody other than the original committer retroactively +1 the patch in gerrit. Just so I can prove everything had extra eyes on it before we deploy the code to payments.
Please let me know if something went wrong with the attachments, and I will gladly resend.
Thanks! -Katie
---------- Forwarded message ---------- From: Elliott Eggleston eeggleston@wikimedia.org Date: Tue, Oct 14, 2014 at 3:48 PM Subject: Fwd: Fwd: For your inner PCI lawyer To: Katie Horn khorn@wikimedia.org
Do this log command and the attached awk script look correct?
git log --no-merges --show-notes=review --name-status fundraising/REL1_22..REL1_23 -- *php *php5 skins resources includes > allLog
./findSelfMerge.awk < allLog > unreviewed.txt
MediaWiki-Core mailing list MediaWiki-Core@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-core
That was very fast. Thank you! I believe we have what we need now to go ahead with the mediawiki upgrade on payments: Reverts and rebases should be just fine, as are backports that were reviewed elsewhere. Hopefully we will be able to work those in to our awk script for next time.
Again, thanks for looking at this so quickly. -Katie On Oct 14, 2014 4:51 PM, "Kunal Mehta" legoktm@wikimedia.org wrote:
Hi,
I commented on a bunch of them with "+1" to mark them as "reviewed", all of them were either reverts, or trival doc changes of some kind.
Some of the ones in the list were properly reviewed, but jenkins failed them, so the original owner rebased and +2'd them. Do those need a re-review as well?
The ones on the top of the list were self-merged backports that were all properly reviewed on master, so I didn't +1 them.
-- Kunal
On 10/14/14 4:07 PM, Greg Grossmeier wrote:
Hey guys,
See this note from Katie. Summary:
- PCI compliance says that the code they deploy to the payments
cluster must have peer-review. 2) There are 35 commits to core that are self-merged with no review (see Elliot's script).
Retroactively +1ing the changes is probably sufficient (after you've reviewed the change, of course :) ).
Deadline appears to be "before November".
Help?
Greg
PS: Katie is cc'd, but I don't think she is on ourlist.
---------- Forwarded message ---------- From: Katie Horn khorn@wikimedia.org Date: Tue, Oct 14, 2014 at 3:59 PM Subject: Self-reviews in core between 1.22 and 1.23 To: Greg Grossmeier greg@wikimedia.org
Hey Greg,
Thanks for looking in to this. Elliott came up with the following awk script and its subsequent output, both attached.
To summarize: PCI rules say that we can not deploy code to payments systems if there were any unreviewed (self-merged) patches, and we found about 35 in the upgrade of mediawiki core that fundraising would like to do before November.
It should be sufficient to have somebody other than the original committer retroactively +1 the patch in gerrit. Just so I can prove everything had extra eyes on it before we deploy the code to payments.
Please let me know if something went wrong with the attachments, and I will gladly resend.
Thanks! -Katie
---------- Forwarded message ---------- From: Elliott Eggleston eeggleston@wikimedia.org Date: Tue, Oct 14, 2014 at 3:48 PM Subject: Fwd: Fwd: For your inner PCI lawyer To: Katie Horn khorn@wikimedia.org
Do this log command and the attached awk script look correct?
git log --no-merges --show-notes=review --name-status fundraising/REL1_22..REL1_23 -- *php *php5 skins resources includes > allLog
./findSelfMerge.awk < allLog > unreviewed.txt
MediaWiki-Core mailing list MediaWiki-Core@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-core
mediawiki-core@lists.wikimedia.org