app icon indicating copy to clipboard operation
app copied to clipboard

An easy way to refresh a pull-request adding a contributor

Open jakebolam opened this issue 6 years ago • 15 comments

Is your feature request related to a problem? Please describe. When multiple contributors are being added, a PR could become stale (i.e not up to date with master/conflicts with the master branch). We should have a way to auto resolve this.

Describe the solution you'd like @all-contributors please refresh this PR

Describe alternatives you've considered Alternatively, we could watch for merge event son pull requests opened by the bot, and refresh the PR: screen shot 2019-01-26 at 3 17 09 pm

Additional context

jakebolam avatar Jan 19 '19 13:01 jakebolam

Another way would be to have a stale bot which would label the issue/PR when it gets past the set threshold and then the AC bot could find out easily which of its PR are stale?

Berkmann18 avatar Jan 19 '19 20:01 Berkmann18

Dealing with merge conflicts was my main reason for dropping all contributors in anticipation of this bot. If you have two PR's open from the bot, it sounds like one of them is going to end up with a conflict when the other is merged.

Is it possible for the bot to work similar to the Renovate bot, which watches the repo and updates it's own PR's whenever they encounter a merge conflict?

erquhart avatar Jan 28 '19 21:01 erquhart

@erquhart that would be awesome! If you've got any pointers on implementing it, we're all ears.

jakebolam avatar Jan 29 '19 03:01 jakebolam

Something like:

  1. Watch for push and pull_request.closed events (if pull_request.closed, make sure merged is true in the payload)
  2. If the event was not for the main branch, eg. master, exit
  3. Fetch the list of open PR's from all-contributors bot via the pulls method, and using the state and head parameters to only fetch open PRs by the bot user. Note: head param didn't seem to work properly in a quick curl test :/, might need to filter list of all open PRs programatically, which will require looping for pagination.
  4. If there are no open PRs by the all-contribs bot, exit
  5. Compare the before and after state of the main branch to determine whether .all-contributorsrc or the configured files (eg. README.md) were changed - if they were not changed, exit
    1. If push event, compare before and head sha
    2. If pull_request event, compare pull_request.base.sha and pull_request.head.sha
  6. For each open PR from the all-contributors bot:
    1. Fetch the individual pull request to get mergeable property in the payload
    2. For any PR where mergeable is null, wait a bit (30s?) and try again, test merge commit is building
    3. If mergeable is true, PR is good to go
    4. If mergeable is false, create a new commit (using the bot comment on top of master using the data from the bot comment on the PR, and force push to the PR branch.

Note that the last step is subject to false positives - PRs that aren't mergeable, but for reasons that have nothing to do with the all-contribs bot. But considering you only get this far if specific files were touched, it would take a bit of a perfect storm to trigger enough false positives to make noise. Probably okay to follow up on this concern in a future release if folks complain.

erquhart avatar Jan 29 '19 05:01 erquhart

@tech4him1 had a great idea that may be even better, providing an option to commit directly rather than opening a PR. Possible?

Mentioned in https://github.com/netlify/netlify-cms/issues/2015.

erquhart avatar Jan 29 '19 18:01 erquhart

@erquhart The master branch is locked so only merged PRs can go through but there's probably a way to get the bot to bypass this.

Berkmann18 avatar Jan 29 '19 20:01 Berkmann18

There's a restrict who can push setting that seems to allow specific accounts to bypass branch protections - if that works the way we need it to, projects that protect master can be required to add the bot user to this list.

erquhart avatar Jan 30 '19 00:01 erquhart

@erquhart the auto-commit is interesting, would need information about users who would be authorized to comment and have a user added.

All for auto-refreshing the PR. Do you think just having PRs non-stale would be enough? e.g. just clicking merge.

jakebolam avatar Jan 30 '19 04:01 jakebolam

I suppose it was more of an issue because we typically had contributors add themselves in the same PR as their first contribution, merging immediately wasn't an option, while it would be with the bot. But if we're merging right away to ensure against conflicts, commit seems more appropriate than pull request.

We could list authorized users in the bot config.

erquhart avatar Jan 30 '19 05:01 erquhart

More to your point, though, I agree that "just merge it" is fair for the mvp of this bot. Commit would be a nice no-hassle option.

erquhart avatar Jan 30 '19 05:01 erquhart

@erquhart Would you mind opening a PR for that?

Berkmann18 avatar Apr 15 '19 13:04 Berkmann18

Would love to, but no bandwidth for it at the moment.

erquhart avatar May 07 '19 14:05 erquhart

Hey, sorry for interrupting. Are there any updates?

Hans5958 avatar Jun 12 '21 13:06 Hans5958

@Hans5958 No work (AFAIK) has been done on this and the team has shrunk since this issue was opened. Feel free to create a PR for this.

Berkmann18 avatar Jun 14 '21 21:06 Berkmann18

So is it safe to say that this boot has been relatively successful with the snapdragon framework for the galaxy a71 5g?

Jesusizalive96 avatar Sep 19 '21 00:09 Jesusizalive96