Closed brettchalupa closed 10 years ago
bcobb
commented
10 years ago The main reason we opted for comments over the commit status API is that only the most recent status is visible through GitHub's UI. If, for instance, Travis takes longer than the CLA Checker, the visible status will be the Travis status, and the only way to see the CLA status is via the API.
bcobb
commented
10 years ago As for checking existing PRs, the ideal solution might be that when a somebody signs a CLA, see if there are any open PRs that Curry has previously seen as unresolved by that user and go put a new message on them. Agreed that that's probably a v2 feature that has lots of unanswered questions.
We were planning on rolling this into v1, but perhaps we haven't uncovered enough open questions yet :grin: What sorts of questions spring to your mind?
sethvargo
commented
10 years ago Statuses cannot be changed once added to a commit (they're completely immutable), but any number of statuses may be attached to a single commit. We only display the most recent status for any given commit in our UI.
:frowning:
What about using a label instead? We definitely want to use a comment for non-existent CLAs, at-tagging the submitter so they get a notification. What about using the API to add a label to the PR once it's verified. Then we can just filter by that label while reviewing. Just a thought. I think a comment would be okay too though.
bcobb
commented
10 years ago I like the label idea! We'll roll with that.
sethvargo
commented
10 years ago @bcobb I was being forced to close my laptop for a plane and didn't get to fully explain. The drawback to labels is that they don't retain any history. But thinking about it, do we care?
bcobb
commented
10 years ago So long as it's not possible to falsely label one's own PR, it seems like it should be OK. On this note, we've just pushed a commit to outline how we'll handle each type of Pull Request event.
sethvargo
commented
10 years ago @bcobb only people with commit-bit can label PRs, so I think we should be fine.
gabeweaver
commented
10 years ago Closing this as it's completed.
Curry checks that committers in a GitHub Pull Request have signed Chef's CLA in Super Market.
Pull Requests that relate to this:
84
Intro
The general goal of curry is to verify that committer(s) opening a pull request in a Chef Inc. repo have signed a CLA. If they have signed a CLA, Super Market will leave a comment letting Chef Inc. know that the user has signed a CLA. If they have not, Super Market will leave a comment letting Chef Inc. and the committer(s) know they have not signed a CLA, while instructing the committer(s) to sign a CLA. When the Pull Request is opened, a ticket is opened in the ticket tracking system being used (in Chef's case, JIRA).
Major parts of Curry include:
Flow diagram of the process.
Information
Implementation Details
Approach
Comments from the original Gist
@gabeweaver said:
One thing I think we should discuss with Chef:
Given that they are moving towards using LeanKit internally, it seems like JIRA is just an added layer of complexity in managing and tracking work to be done. I'm assuming Chef will be using the "Portfolio" version on LeanKit. If that's the case, then they support a really nice GitHub integration:
And since PRs also create GitHub issues, it seems like a Win/Win for reducing complexity and simplifying the workflow, not to mention reducing the barrier to entry and things for contributors to track.
@sethvargo said:
Can we use the commit status API instead? I think a comment is intrusive.
&&
Also, we probably want a batch job that goes through existing PRs and "rechecks" the status. Maybe that's a v2 though
@smith said:
I'm with @sethvargo on using the commit status API. Are there any reasons why we wouldn't do that and use comments instead?
As for checking existing PRs, the ideal solution might be that when a somebody signs a CLA, see if there are any open PRs that Curry has previously seen as unresolved by that user and go put a new message on them. Agreed that that's probably a v2 feature that has lots of unanswered questions.
IMO it might be preferable to leave JIRA completely out of this release, since there might be a lot of complexity around the avoidance of opening duplicate issues. All of the linking we now do between JIRA and GitHub is now manual, and it may have to stay that way for a while.
The LeanKit/GitHub integration sounds really cool, but lets learn how to make a board and use LeanKit before we get there.
Overall this all looks sound, and it a huge improvement over having a human do it.
@gabeweaver @sethvargo @smith I have moved the content and comments from the original Gist to here. I think GitHub Issues are a better place to document discussion because we will get notified of comments, they are tied to the repository and and the discussions should be easier to find.