Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: post comment when previously-denied user accepts CLAs #310

Closed
dfawley opened this issue Oct 31, 2019 · 3 comments
Closed

Feature: post comment when previously-denied user accepts CLAs #310

dfawley opened this issue Oct 31, 2019 · 3 comments
Labels
04 - Med Low Medium-Low Priority (Lower than Medium; higher than Low) enhancement New feature or request size:TBD Size is too be determined after analysis
Milestone

Comments

@dfawley
Copy link

dfawley commented Oct 31, 2019

The focus of #269 was changed after it was created. The new version was implemented and alleviated some of the problems of managing CLA issues, but not all of them. Hence, I am reposting the original feature request in a new issue.


Summary

Post comment when previously-denied user accepts CLAs.

Background

If a contributor has not accepted the CLAs when creating a PR in a managed repo, the CLA bot will post a message indicating this. If the user then accepts the CLAs, this comment will be changed to indicate they have accepted the CLAs. However, if I, as a maintainer, was waiting for this action in order to merge the PR, no update will be sent when this happens, and I will have to poll the PR to find out it's ready to merge.

The mention in this comment that grpc maintainers didn't want an update for a user that has accepted the CLAs was in regards to users that were already OK when the PR was created. We still wouldn't want messages in these cases. Only after the bot has posted a comment indicating the CLAs were denied would we want an update.

User Story

As a maintainer, I would like a new comment to be posted when a previously-denied contributor accepts the CLAs in order to avoid the need to poll the PR to discover its state. Note that this is in-line with how the previous CNCF CLA bot worked

cc @jpalmerLinuxFoundation

@dealako dealako added 04 - Med Low Medium-Low Priority (Lower than Medium; higher than Low) enhancement New feature or request labels Nov 26, 2019
@dealako dealako added the size:TBD Size is too be determined after analysis label Nov 26, 2019
@pranab-bajpai pranab-bajpai added this to the Sprint 09 milestone Dec 3, 2019
@dfawley
Copy link
Author

dfawley commented Dec 11, 2019

This situation happened to us in grpc/grpc-go#3231. The only reason I noticed the user agreed to the CLAs was by looking at the open PR list and saw an approved and green CL ready to submit. In our repo, this won't usually result in a significant problem as we closely monitor all of our open PRs. In repos that have a large number of outstanding PRs, this could easily lead to PRs not being noticed for weeks after they are ready, causing the author to need to rebase and fix merge conflicts.

@dealako
Copy link
Member

dealako commented Dec 1, 2020

As per stakeholder feedback, a comment is added when EasyCLA fails the status check. Later, when the user or users are authorized and the EasyCLA status check passes, the original comment is updated. No comment is added when the user passes the EasyCLA check when the PR is originally opened.

@dealako dealako closed this as completed Dec 1, 2020
@dfawley
Copy link
Author

dfawley commented Dec 1, 2020

This FR is to make it so a note is posted when the user passes the EasyCLA check after previously failing it. I agree we don't need a comment if they are already passing when the PR is originally opened. This is something I would still like to have, as our workflow is:

  1. PR comes in from user that has not accepted CLA.
  2. PR labeled as "requires action from author".
  3. Author completes CLA process.
  4. ????
  5. We eventually notice (3) and then review/merge the PR.

Step 4 needs a notification so we know when to proceed to step 5 without resorting to polling. Right now it only updates the previous post, which doesn't trigger an email or any other push notification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
04 - Med Low Medium-Low Priority (Lower than Medium; higher than Low) enhancement New feature or request size:TBD Size is too be determined after analysis
Projects
None yet
Development

No branches or pull requests

3 participants