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

.github/workflows/periodic-merge: generalize from merge-staging #128300

Merged
merged 1 commit into from
Jun 30, 2021

Conversation

mweinelt
Copy link
Member

@mweinelt mweinelt commented Jun 27, 2021

Motivation for this change

By generalizing the previous merge-staging action we can support a large
number of branch pairs that need to be merged periodically.

Tested over here:
https://github.com/mweinelt/nixpkgs/actions/workflows/periodic-merge.yml
mweinelt#6

I only created haskell-updates to test that merging still works, I don't have any of the staging-next branches, so they obviously fail.

https://github.com/mweinelt/nixpkgs/actions/runs/976153168

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Relase notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@cdepillabout
Copy link
Member

I was talking about this with @maralorn on Matrix, but maybe it makes sense to post it here as well.

Making this run every 6 hours means that it will run 4 times a day. This means that people subscribed to these PRs could potentially get pinged an additional 4 times a day. This seems a little excessive, since these updates are mostly non-actionable and not interesting.

Is there any way to work around this, or possibly drop the frequency to once per day?


This affects me personally in the haskell-updates PRs (like #127833). Merging master into haskell-updates is mostly uninteresting (and I don't want to get notified about it), except in the case where staging-next has been merged into master. I really don't want to increase my GitHub notifications, but I also don't want to have to unsubscribe from these PRs that I should be watching.

@jonringer
Copy link
Contributor

@cdepillabout or just accept the avalanche >:)

Selection_003

@Mic92
Copy link
Member

Mic92 commented Jun 28, 2021

I was talking about this with @maralorn on Matrix, but maybe it makes sense to post it here as well.

Making this run every 6 hours means that it will run 4 times a day. This means that people subscribed to these PRs could potentially get pinged an additional 4 times a day. This seems a little excessive, since these updates are mostly non-actionable and not interesting.

Is there any way to work around this, or possibly drop the frequency to once per day?

This affects me personally in the haskell-updates PRs (like #127833). Merging master into haskell-updates is mostly uninteresting (and I don't want to get notified about it), except in the case where staging-next has been merged into master. I really don't want to increase my GitHub notifications, but I also don't want to have to unsubscribe from these PRs that I should be watching.

Does this not only trigger a notification if the merge failed?

@sternenseemann
Copy link
Member

sternenseemann commented Jun 28, 2021

Does this not only trigger a notification if the merge failed?

You get one email per commit, usually.

@mweinelt
Copy link
Member Author

@cdepillabout or just accept the avalanche >:)

Selection_003

Being a code-owner for > 4.3k python "modules" doesn't sound like a sensible thing to begin with. Maybe drop that and things will get back to normal? You can still filter for topic: python when you have the time and wish to check up on the backlog.

@mweinelt
Copy link
Member Author

Making this run every 6 hours means that it will run 4 times a day. This means that people subscribed to these PRs could potentially get pinged an additional 4 times a day. This seems a little excessive, since these updates are mostly non-actionable and not interesting.

Maybe unsubbing from "Pull Request Pushes" helps?

image

@cdepillabout
Copy link
Member

Maybe unsubbing from "Pull Request Pushes" helps?

If this gets merged in as-is, that might be what I'll have to do.

Although, in general, I do want to see pull request pushes. However, the ones triggered from this PR would be both very(?) frequent and mostly uninteresting / non-actionable.


I sent another message about this on Matrix, but I guess I should post it here as well.

I want to be clear that I'm not opposed to this automatic merging workflow at all, just the amount of notifications that would be triggered by the frequency of the merging.

Is there some sort of argument for doing the merges 4 times a day? It seems somewhat arbitrary to me, and I can't think of a reason why we wouldn't instead go with, for example, 6 times per day (more frequent), twice per day (slightly less frequent), or once per day (less frequent). Is 4 times a day a sweet spot?

In the case of haskell-updates, it seems like once-per-day (or once-every-other-day?) would be a good compromise. I'm not sure about the other branches though.

@jonringer
Copy link
Contributor

Being a code-owner for > 4.3k python "modules" doesn't sound like a sensible thing to begin with. Maybe drop that and things will get back to normal? You can still filter for topic: python when you have the time and wish to check up on the backlog.

When I was most active, I was actually able to stay on top of the 100-200 notifications I would get a day. Obviously that's not the case anymore ;)

@mweinelt
Copy link
Member Author

Is there some sort of argument for doing the merges 4 times a day? It seems somewhat arbitrary to me, and I can't think of a reason why we wouldn't instead go with, for example, 6 times per day (more frequent), twice per day (slightly less frequent), or once per day (less frequent). Is 4 times a day a sweet spot?

Likely has something to do with staging-next being evalued every 12 hours. I don't have an idea how we would be able to cover multiple different cron jobs without creating multiple separate workflows.

@maralorn
Copy link
Member

The Haskell Team has finished it’s timezone spreading discussion and we have found a conclusion. We would like master -> haskell-updates merges to happen every 24h.

Now I see a few ways forward:
a) Remove the haskell-updates listing from this pr and create a second file with another rhythm.
b) change the rhythm in this file, but I guess that won‘t fit for all use cases
c) Build a feature with conditionals into this workflow so that some jobs are not run everytime. Maybe other merges also would profit from that feature?

I am okay with either of those.

@jonringer
Copy link
Contributor

a) Remove the haskell-updates listing from this pr and create a second file with another rhythm.

Instead of domain specific files, what about just having different files which are denoted by their frequency (e.g. daily-merges.yml, six-hour-merges.yml)?
As new workflows arise, then the ecosystem can just choose the related period.

By generalizing the previous merge-staging action we can support a large
number of branch pairs that need to be merged periodically.

Provide two intervals, daily and every six hours, to accomodate
different needs.

Co-Authored-By: Malte Brandy <[email protected]>
@mweinelt
Copy link
Member Author

Updated.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maralorn maralorn merged commit 6276ad0 into NixOS:master Jun 30, 2021
@mweinelt mweinelt deleted the actions/periodic-merge branch June 30, 2021 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants