-
Notifications
You must be signed in to change notification settings - Fork 9
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
Allow reviewers to tell marvin to opt-in #68
Comments
Because only the PR author can opt-in for now. You don't need to be part of the reviewer team to opt-in, but you do need to be the author. |
I see. Considering that now marvin is a bit new and perhaps not many people know about it, perhaps would it be better to allow anyone to tell marvin to opt-in? For my current usage, I really only find it useful to mark PRs I reviewed as ready so I'm only interested in the labels. |
Well in a way opt-in is restricted to PR authors precisely because Marvin is so new. I want to carve out some little safe corner of nixpkgs to experiment, while affecting the rest of it as little as possible. That reduces the potential for bugs and for having people form negative opinions about it before it is finished. Especially in the very beginning, it makes sense to restrict that "corner" to PRs where the author knows what is going on and what they are signing up for. Another reason is that I am still monitoring most interactions with marvin, and from that perspective the "organic" review cycles are just more interesting than a simple "opt-in -> needs_merger". I might expand the beta at some point, but before that we will have to reach a point where continuous monitoring is less necessary and I will have to restrict my "merger rate-limit" (which is currently unlimited) and maybe find some other reviewers with commit permission. Until then, feel free to "convert" the PR author to opt-in themselves and maybe get on board with future PRs as well ;) |
Is there a typo in NixOS/nixpkgs#92152 (comment) ? Should I be part of the team?
The text was updated successfully, but these errors were encountered: