Owners File Recognition/Reward Bot

[parispittman]

What would you like to be added:
A bot that will be kicked off when a PR gets merged adding someone (individual; not an alias) to an owners file as a reviewer or approver. Content will be:

• a congratulations,
• links to important documentation for reviewers/approvers like community expectations/responsibilities/tips, and
• instructions on how to get a promo code to order a free contributor patch from the cncf store (email contributors@kubernetes.io with the PR link - or could this be automated too? :) ).

Why is this needed:
we don't reward and recognize our contributors enough in a scalable way, especially our owners who do countless reviews, etc. we also need important community governance and operational docs to be surfaced to these folks.

[nikhita]

/sig contributor-experience

The plugin adds a "positive emoji" whenever an owner is added:

test-infra/prow/plugins/heart/heart.go

Lines 140 to 148 in f7e3d43

	if (filename == ownersFilename || filename == ownersAliasesFilename) && change.Additions > 0 {
	c.Logger.Info("Adding new OWNERS makes me happy!")
	return c.GitHubClient.CreateIssueReaction(
	pre.PullRequest.Base.Repo.Owner.Login,
	pre.PullRequest.Base.Repo.Name,
	pre.Number,
	reactions[rand.Intn(len(reactions))])
	}
	}

I think this is a cool idea :) but just thinking out loud...would it be weird if we are removing more folks than adding? 🙈 🤔

[nikhita]

instructions on how to get a promo code to order a free contributor patch from the cncf store

To be clear, this is only if the contributor doesn't already have the patch, right? Just to make sure we get the message in the bot comment right 😬

[parispittman]

they can collect more than one patch if they wish or ask for another item and if we have the inventory we can provide

[parispittman]

/help

[k8s-ci-robot]

@parispittman:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[spiffxp]

/remove-lifecycle stale
linking to the issue I'm using as an umbrella for all things OWNERS/contributor-ladder related kubernetes/community#1808

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[parispittman]

/remove-lifecycle stale

still needs to get done :)

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[parispittman]

/remove-lifecycle stale

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[nikhita]

/lifecycle frozen

…

On Sun, Apr 19, 2020 at 11:35 PM fejta-bot ***@***.***> wrote: Issues go stale after 90d of inactivity. Mark the issue as fresh with /remove-lifecycle stale. Stale issues rot after an additional 30d of inactivity and eventually close. If this issue is safe to close now please do so with /close. Send feedback to sig-testing, kubernetes/test-infra and/or fejta <https://github.com/fejta>. /lifecycle stale — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#11994 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AD24BUA7MGI7FBGNSURUFTTRNM4PHANCNFSM4HCGPCIA> .

[matthyx]

@nikhita @parispittman do you still need a hand at this?
Should it be a new plugin, or rename+modify heart?

[matthyx]

/assign

[matthyx]

Content will be:

• a congratulations,
• links to important documentation for reviewers/approvers like community expectations/responsibilities/tips, and
• instructions on how to get a promo code to order a free contributor patch from the cncf store (email contributors@kubernetes.io with the PR link - or could this be automated too? :) ).

@parispittman could you give an example... I think the text+links could go to config/prow/plugins.yaml

@cjwagner do you think it's wise to reuse heart.go or should I put this in a different plugin?
I think it really depends if we wanna do something generic or specific to k/k

[cjwagner]

It seems like this should be easy to make configurable and generic for use outside k/k. I think this would be a reasonable extension to the heart plugin if you'd like to integrate there, but as Nikhita mentioned the logic for detecting additions to OWNERS is currently pretty simple and should probably be updated (e.g. distinguish adding users from adding labels).

[matthyx]

@cjwagner I wonder if it wouldn't be easier to react inside verify-owners as we already load the files and parse the added owners...
I also wonder whether we should not remove heart completely, even though it makes a simple example of a prow plugin.

[matthyx]

@parispittman and @nikhita, couple of questions:

• should we comment inside the PR adding the new reviewer/approver, or we contact them differently (mail, slack, …)?
• should we check whether the individual is a trusted user (i.e. member of the org)?
• should we add a specific label to the PR to identify the PRs later?
• what happens if the individual is moved from reviewer -> approver?
• what happens if the individual was already reviewer/approver elsewhere in the same repo?
• what happens if the PR is modified and no new owner is added, should we purge the welcome message?

[cjwagner]

@cjwagner I wonder if it wouldn't be easier to react inside verify-owners as we already load the files and parse the added owners...

Sharing code with verify-owners would be fine, but I don't think we should add recognition/reward logic to the validation plugin, those a pretty different roles and it would be nice to enable/configure them separately

I also wonder whether we should not remove heart completely, even though it makes a simple example of a prow plugin.

Unless it's too expensive, which I don't think is the case, I'd like to keep the heart plugin. It is simple and fun. I don't think deprecating and removing it would be a productive use of time.

should we check whether the individual is a trusted user (i.e. member of the org)?

IIRC verify-owners will guarantee this and can be assumed to be enabled since check-config warns if approve or blunderbuss are enabled without it.

[matthyx]

@cjwagner I have started with something here: #19449

[matthyx]

@parispittman any update on the text+links to include to the comment?

[parispittman]

content:
Thanks for serving the community in this new capacity!

Next steps:
1- Reach out in #sig-contribex for your special badge swag!

2- Join #kubernetes-contributors in slack and dev@kubernetes.io for all upstream info

3- Review the community-membership.md doc for your role. If for some reason you can't perform the duties associated, Emeritus is a great way to take a break! OWNERs is another great resource for how this works.

4- Look over our governance docs now that you are actively involved in the maintenance of the project.

[kikisdeliveryservice]

maybe to make sure you are in line with the duties -> to confirm that you are able to fulfill the duties?

Also: is that governance link correct?

[parispittman]

@matthyx I think we are finally ready!! LOL I cleaned up the content in the comment above. thank you so much for your patience and sticking with me :)

[matthyx]

@matthyx I think we are finally ready!! LOL I cleaned up the content in the comment above. thank you so much for your patience and sticking with me :)

Let me rebase the PR and we can move on.

[matthyx]

I had to open another PR... Let's do it now!

[matthyx]

Woot woot @parispittman it's merged!