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

tasks/evaluate: error on PRs targeting nixos-* or nixpkgs-* branches #546

Merged
merged 1 commit into from Jan 20, 2021

Conversation

cole-h
Copy link
Member

@cole-h cole-h commented Jan 16, 2021

A PR's target branch is easy to overlook. ofborg should complain loudly
if a nixos-* or nixpkgs-* branch is the target of a PR.

NixOS/nixpkgs#109384, https://logs.nix.samueldr.com/nixos/2021-01-14#4470635


Happy to bikeshed on the actual message, but I think this is the right thing to do (and probably should have been done long ago).

EDIT: pedantry fixed here: #547

Copy link
Member

@LnL7 LnL7 left a comment

Choose a reason for hiding this comment

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

Hmm, I seem to remember that merging to channel branches didn't work. Did something change or is this perhaps something explicit per branch that wasn't updated with 20.09?

@cole-h cole-h force-pushed the nixos-nixpkgs-bad branch 2 times, most recently from 8e95a43 to 080e3e7 Compare January 16, 2021 22:20
@cole-h
Copy link
Member Author

cole-h commented Jan 16, 2021

Don't mean to shame anyone, but the person who merged the PR to nixos-20.09 was an Owner in the org (and thus was able to bypass the protected branch restriction). It's easy to overlook, so I think it makes sense to raise an alarm, especially for cases like these.

@LnL7
Copy link
Member

LnL7 commented Jan 16, 2021

Ah that explains it. Either way this is an extra place to catch it and it's something it gives feedback to the contributor. Perhaps the message could even be improved to give a bit more detail on what's going on with some of the information from eg. https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md#backporting-changes.

Currently it's up to the reviewer to explain https://github.com/NixOS/nixpkgs/pulls?q=base%3Anixos-20.0

ofborg/src/tasks/evaluate.rs Outdated Show resolved Hide resolved
A PR's target branch is easy to overlook. ofborg should complain loudly
if a nixos-* or nixpkgs-* branch is the target of a PR.
@cole-h
Copy link
Member Author

cole-h commented Jan 20, 2021

Even if NixOS/nixpkgs#109543 is accepted and merged, I'd also like to have a check in ofborg itself.

@cole-h cole-h merged commit f56bea3 into NixOS:released Jan 20, 2021
@cole-h cole-h deleted the nixos-nixpkgs-bad branch January 20, 2021 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants