Automatic Merging Implementation

[grahamc]

I would add the following command:

@GrahamCOfBorg build-and-merge foo bar baz

Then, if:

• You are unknown: the command is ignored
• You are a known or trusted user: the builds are queued
• You are a Nixpkgs member (a subset of the known users): a merge job
  is queued

If a merge job is queued, it will contain:

• a "command ID". All build jobs from this command will carry the same
  command ID.
• a list of build jobs triggered to wait for

The merge job will then wait for each build job to complete.

If any return a Failure, the merge job is canceled.

If the PR's status checks fail, the job is canceled.

If any known or trusted user comments:

@GrahamCOfBorg cancel

after the command was issued, the job is canceled.

When all of the jobs complete with Success or "not attempted" (a special
status to indicate the job wasn't possible, like building systemd on
x86_64-darwin) the PR is then merged by OfBorg's user. The merge
commit will indicate who issued the merge command.

[infinisil]

I think it would be a good idea for it to have a delay before the actual merge is done. So it would post a comment saying "Will automatically merge in 1 hour without intervention". Then people have some time to react before a bot changes master all by itself.

[grahamc]

Note: At least one job must succeed. If all jobs are Not Attempted, then the merge job is canceled.

[shlevy]

👍 from me, with or without @infinisil's addendum (which I agree is better).

[7c6f434c]

I think the initial implementation as described by @grahamc would be nice, then later and separately ofborg could also learn something like build-and-merge-after 24h foo bar baz, then original command could become an alias for the delayed merge after build with some default delay to discuss later.

Also, cancellation should be available not only to all known (and trusted) users, but also to the original submitter, and ideally package maintainer — maybe the first iteration should make it available to everyone, because the person requesting the merge will see a notification and can merge manually if needed.

[shlevy]

I wonder how long it will be before ofborg gets is own UI...

[Mic92]

I think a UI is already work-in-progress.
In future also a eval-and-merge would be nice to avoid things like: NixOS/nixpkgs@b8ebbc0#r28046806

[grahamc]

eval-and-merge is an even easier place to start...

[vcunat]

Specifying an additional delay might be deferred to another iteration of this feature. I guess sometimes the PR seems OK, but I may still want to give the package maintainer(s) a day or two to react if they want, depending on the case.

[timokau]

To avoid confusion it would be good to post a quick explanation when a command is ignored (when a non-trusted or non-commit user issues the command).

[grahamc]

@timokau please open a new issue for that, because it is a problem with all commands not just this one.

[obadz]

It seems the 24h (optional) delay seems to really be popular in people's comment on this topic.

I think the reason is: a lot of us feel like a PR is OK, but we're not necessarily 100% confident about it, or really we think there must be someone out there with more expertise and authority to review this.

The reality is, there is probably someone with more expertise but that someone may never find the time to review the PR. merge-if-build-after-24h gives us shy reviewers an easy conscience because that theoretical expert reviewer now has the option to act.

Also, would be nice if ofborg tagged the PR as "slated-for-merging" when the command is issued so that the PRs can be filtered from workflow?

[7c6f434c]

I think the reason is: a lot of us feel like a PR is OK, but we're not necessarily 100% confident about it, or really we think there must be someone out there with more expertise and authority to review this.

I would expect «PR seems OK, but we might be missing the perspective from people with different use cases.» to be a more important argument. Unlike qualification and authority this is unavoidable even in principle.

Also, would be nice if ofborg tagged the PR as "slated-for-merging" when the command is issued so that the PRs can be filtered from workflow?

I agree. Adding lables seems to be a relatively easy part, though.

[domenkozar]

Done?

https://github.com/sindresorhus/refined-github

[7c6f434c]

@domenkozar No, this is not enough to distinguish must-never-fail checks and build-checks that require some value judgements. I want to be able to merge SBCL conditional on Maxima building fine (maybe Maxima tests passing); but maybe Maxima build should not be a blocker if it was already tested locally.

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/bootstrap-files-updates-amplifiy-exploit-of-any-package-into-exploit-of-every-package/50534/6