Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
.github/workflows/labels.yml: set event types
opened, synchronize, reopened are the defaults for `pull_request_target`, `edited` will trigger the label action if the PRs base branch is changed.
- Loading branch information
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please open a PR for these in the future.
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are direct pushes disallowed now?
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very strongly discouraged.
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's very common practice on master and cherry picking to stable. Why comment on this commit?
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I comment on most of them. It is not very common practice.
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cherry-picking to stable is a different workflow than introducing new work. When cherry-picking to stable you're importing already approved changes in another branch.
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't say it wasn't.
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anyway, please use PRs and don't push to master directly.
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@grahamc Can you open an issue about this so we have something that is more visible and easier linked to?
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some committers pushing to master still seems to be their standard method for updating packages.
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @zowoq, that's currently the case, but we're (slowly) moving towards using PRs.
There was already RFC proposed at NixOS/rfcs#79 that wasn't accepted as we need to figure out some stuff first.
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm aware on the RFC and that it wasn't accepted which is why I wanted clarification of the very brief comments:
For the record: I don't have a problem with the PR workflow for master/staging/etc.
If we're going to block pushing cherry picks to the release branch I probably won't bother doing many backports.
It's just very odd that an attempt is being made to enforce a PR workflow via comments on commits without any context?
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We warn people that push directly, sometimes via IRC, sometimes via commit comments, etc.
I don't think this is ideal, but it's still better to communicate what's the consensus where we'd like to end up, without the consensus how to get there.
Using github cli that should be one extra command to run.
I agree there should be a link with more explanation, so I opened #118661
574c4a7There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To open a PR. It still requires more steps to land it in the release branch.
Sure it's a trivial amount of time and effort but I basically don't care about stable so I'm not going to spend more time and effort on it than I currently am.
I'm only doing backports on some packages I maintain when I update them (e.g. the github cli tool) because it is low effort and I'll get pinged for issues and backport PRs anyway.
But I'm not looking to campaign against and/or otherwise try to impede the "no direct pushes" effort, as I said I'm fine with it for master/staging and I'll probably just ignore stable.
Thanks, appreciated.
Pinging the
nixpkgs-committersteam on the issue may be a better method?