Is it ok to review and approve my own PR?

Development
#1

If I do a simple version bump of a package I maintain, with no other changes, just like how the automated @r-ryantm would, is it ok for me to nixpkgs-review the PR, including submit-result and approve?

It feels a little icky to do that, but I wonder whether it would help by signalling to a committer that the PR is ready for merge, reduce the overhead on other contributors by removing it from the list of PRs waiting for review, and of course speed up getting the new version into unstable.

2 Likes
#2

We don’t have an official policy so it will vary between the people you ask.

Personally, I self-merge if I am the only maintainer on the package and the change is trivial, and it doesn’t cause a mass rebuild. I would still run nixpkgs-review on it before merging.

If there is another maintainer, I would leave a couple of days, with some sort of multiplier depending on the level of change on the package. 5-7 days for a version bump. Two weeks for something bigger.

4 Likes
#3

I usually wait a couple days even if there’s no other maintainer, assuming the change is nontrivial and isn’t urgent. Sometimes other people review, too.

4 Likes
#4

nixpkgs-review is already enough. If it is only one rebuild ofborg is already enough. Then you only need to find someone to merge which can be the harder part.

1 Like
#5

Thanks for your comments everyone.

I always thoroughly test the PR for packages I maintain before even submitting them. So given that, I’ll nixkpkgs-review my simple version bump PRs from now on to signal I’m confident in it, and see how that goes.

1 Like
#6

In addition to all the good points raised by the others in this thread - there’s one thing I’d like to add.

I have been surprised again and again by the quality feedback I have gotten on what looks like extremely simple changes. Feedback I would have missed out on if I didn’t take the time to wait for it. We (or maybe just I) tend to get so focused on the solution especially when it’s small and obvious, that we lose track of the problem we’re trying to solve.

I would argue as a rule of thumb, that unless the change is to unbreak the build, that it should be left for a period of time to solicit input.

6 Likes
Hosted by Flying Circus.