Disallowing at mentions and keywords which close issues in commit messages across all Kubernetes GitHub orgs

32 views
Skip to first unread message

Nikhita

unread,
Apr 16, 2019, 10:01:27 PM4/16/19
to kubernetes-dev, kubernetes-sig-contribex
Hey all,

When a commit message containing at mentions (example: @nikhita) or keywords which close issues (example: fixes #XXX) is pulled into a fork, GitHub might take action on these keywords by:

- Pinging the person who is at mentioned, leading to excessive emails.
- If the person has write access to a repo, it will close issues that may not be ready to be closed.

Note that the keywords have these problems only if they are mentioned in the commit message - mentioning them in the PR body is fine.

To avoid these scenarios, we intend to enable the "invalidcommitmsg" prow plugin across all active Kubernetes GitHub orgs in this PR: https://github.com/kubernetes/test-infra/pull/12236.

If a PR has commits with messages containing these keywords, this plugin will automatically add the `do-not-merge/invalid-commit-message` label and add a comment specifically mentioning invalid commits so that they can be fixed.

Lazy consensus timeboxed to Friday, April 19, 5pm PT. Please chime in on the PR if you have any questions, concerns or objections.

Thanks,
Nikhita

Davanum Srinivas

unread,
Apr 17, 2019, 6:25:33 AM4/17/19
to Nikhita, kubernetes-dev, kubernetes-sig-contribex
+1 yes please!

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-contribex" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-con...@googlegroups.com.
To post to this group, send email to kubernetes-s...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-sig-contribex/CAFPyxn%2BNj_zvo2XoZvkMbHhMg9-R_kOzcGr2Mwnn3ks3UXsmvQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


--
Davanum Srinivas :: https://twitter.com/dims

Brian Grant

unread,
Apr 17, 2019, 11:38:32 AM4/17/19
to Davanum Srinivas, Nikhita, Kubernetes-dev, kubernetes-sig-contribex
Thank you!

You received this message because you are subscribed to the Google Groups "Kubernetes developer/contributor discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-de...@googlegroups.com.
To post to this group, send email to kuberne...@googlegroups.com.
Visit this group at https://groups.google.com/group/kubernetes-dev.
To view this discussion on the web visit https://groups.google.com/d/msgid/kubernetes-dev/CANw6fcH2Qu-Ttfs4WzK8OirkudJsoyCBo4X76JoZwDCgSYQANg%40mail.gmail.com.

Josh Berkus

unread,
Apr 17, 2019, 12:25:31 PM4/17/19
to Nikhita, kubernetes-dev, kubernetes-sig-contribex
On 4/16/19 7:00 PM, Nikhita wrote:
> Note that the keywords have these problems only if they are mentioned in
> the commit message - mentioning them in the PR body is fine.
>

I thought that having "Fixes #9999" in the PR body would *also* cause
the issue to close automagically. No?

--
--
Josh Berkus
Kubernetes Community
Red Hat OSAS

Josh Berkus

unread,
Apr 17, 2019, 12:30:49 PM4/17/19
to Jordan Liggitt, Nikhita, kubernetes-dev, kubernetes-sig-contribex
On 4/17/19 9:26 AM, Jordan Liggitt wrote:
> It does, and that is desired. It is the spooky action at a distance as
> commits get pulled into unrelated forks/repos that is not desired.

Speaking as a member of the Release Team, we'd like to *also* disable
auto-close for some PRs in the body, but that sounds like it's a
different feature.

For example, we don't want e2e/conformance test fix PRs to auto-close
anything; those issues shouldn't close until the test is confirmed passing.

Josh Berkus

unread,
Apr 17, 2019, 12:44:42 PM4/17/19
to Jordan Liggitt, Nikhita, kubernetes-dev, kubernetes-sig-contribex
On 4/17/19 9:30 AM, Josh Berkus wrote:
> On 4/17/19 9:26 AM, Jordan Liggitt wrote:
>> It does, and that is desired. It is the spooky action at a distance as
>> commits get pulled into unrelated forks/repos that is not desired.
>
> Speaking as a member of the Release Team, we'd like to *also* disable
> auto-close for some PRs in the body, but that sounds like it's a
> different feature.
>
> For example, we don't want e2e/conformance test fix PRs to auto-close
> anything; those issues shouldn't close until the test is confirmed passing.
>
Oh, and to be clear: +1 on the commit message change.

Erick Fejta

unread,
Apr 17, 2019, 12:59:35 PM4/17/19
to Josh Berkus, Jordan Liggitt, Nikhita, kubernetes-dev, kubernetes-sig-contribex
+1 this is a fantastic change. These types of messages cause confusion and annoyance (especially as people push the commits into their forks). Will be great to help people not make them.


> we'd like to *also* disable auto-close for some PRs in the body

FYI I know that many of us already know this, but all of this action is done by github. It is not behavior we coded into our bots.

This is unfortunate because when k8s-ci-robot pushes a commit with a "fixes #123" comment github "helpfully" closes #123 and adds a message on #123 saying that k8s-ci-robot closed the issue, which is mostly untrue and not something it has a choice over. But it causes confusion and we get the occasional support question about it.

This is different from what happens when someone leaves a comment to an issue with a "/close" line. In this scenario it triggers code that we wrote and control that causes the bot to validate the request and call the github's close api on that issue. We can change this behavior as we need. Unfortunately, we cannot control what github does with a fixes #123 comment/commit.


--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-contribex" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-con...@googlegroups.com.
To post to this group, send email to kubernetes-s...@googlegroups.com.

Josh Berkus

unread,
Apr 17, 2019, 1:28:08 PM4/17/19
to Erick Fejta, Jordan Liggitt, Nikhita, kubernetes-dev, kubernetes-sig-contribex
On 4/17/19 9:59 AM, Erick Fejta wrote:
> +1 this is a fantastic change. These types of messages cause confusion
> and annoyance (especially as people push the commits into their forks).
> Will be great to help people not make them.
>
>
>> we'd like to *also* disable auto-close for some PRs in the body
>
> FYI I know that many of us already know this, but all of this action is
> done by github. It is not behavior we coded into our bots.

I want to move this discussion to an issue, so that we don't drag out
the approval of nikhita's very helpful patch.

https://github.com/kubernetes/test-infra/issues/12240

Aaron Crickenberger

unread,
Apr 17, 2019, 4:57:36 PM4/17/19
to Josh Berkus, Erick Fejta, Jordan Liggitt, Nikhita, kubernetes-dev, kubernetes-sig-contribex
Explicit +1 for enabling the invalid commit message plugin across all kubernetes orgs

- aaron

--
You received this message because you are subscribed to the Google Groups "kubernetes-sig-contribex" group.
To unsubscribe from this group and stop receiving emails from it, send an email to kubernetes-sig-con...@googlegroups.com.
To post to this group, send email to kubernetes-s...@googlegroups.com.

Benjamin Elder

unread,
Apr 17, 2019, 6:59:19 PM4/17/19
to Aaron Crickenberger, Josh Berkus, Erick Fejta, Jordan Liggitt, Nikhita, kubernetes-dev, kubernetes-sig-contribex

Christoph Blecker

unread,
Apr 20, 2019, 12:00:12 AM4/20/19
to Benjamin Elder, Aaron Crickenberger, Josh Berkus, Erick Fejta, Jordan Liggitt, Nikhita, kubernetes-dev, kubernetes-sig-contribex
This is now merged and active. Thanks everyone!

Reply all
Reply to author
Forward
0 new messages