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
Proposal: Allow more PRs without issues #7015
Comments
I'm all for not requiring issues for Build Related/Chores/Upgrades tasks - they always feel redundant. One thing having an issue for PRs provides is a separation between discussions around what the change should be and the actual implementation. Do you envision the discussion for said issue would all happen in the PR? For example, when a PR comes in for a rule enhancement, would we discuss on the PR and label
From the cons list - this was my first thought when reading this, and I think is a legitimate concern. |
Just read through this again and I think I missed the point a bit initially. In that light, I have followup thoughts:
|
I'm not sure I like the idea of requiring a PR for a new rule either. I agree it reflects the reality of ESLint (we can't just implement new rules for everyone), but it goes back to the con of "I spent all this time on a PR and it got rejected, what gives?". I think there should still be a relatively risk-free way for people to ask, is this rule something ESLint would even consider. Maybe that way is simply to ask users to visit the chat room, though, I don't know. 👍 to everything else. |
Yup, definitely. However, our history seems to indicate that it's rare for a user-suggested rule to be implemented by someone other than the person requesting it. Id actually be fine if we didn't add any new rules for a while, so this isn't a big deal to me. We are trying to push people towards custom rules, and I see this as a step in that direction.
Yup
Can you explain the concern? In theory, people are already looking at both on a regular basis. @platinumazure risk-free way to ask is the chatroom. Most rule requests are "will you build this for me?" rather than "will you accept a PR for this?". Everyone - let's not get stuck on just one type of issue right now. This proposal is large, so please give your thoughts on all parts. |
I'd like to stop using "do not merge" and just use "evaluating" like we do for issues. Same issues process, just on PRs. |
I tend to go through issues and PRs separately when I'm making the rounds, and I think I was having a hard time breaking out of that mindset and worried about having to figure out which PRs "are Issues" and which PRs "are just PRs". The more I think about it, the more that seems like an arbitrary distinction I was making (due to the workflow I've developed), rather than a real problem. Retracting my concern on that point - I'll adapt! :) |
I think this is a great way to reduce the number of open loops, and I think you can simplify the rules around when to open an issue vs. PR.
Contributors should understand that a PR is considered guilty until proven innocent. The team is not obliged to accept it. If the rationale and general approach is not explained in the PR, no one should be expected to review code changes. Changes to the core and breaking changes get more scrutiny, but I don't see any value in having that scrutiny take place in an issue rather than a PR -- especially considering that we often don't know a change is breaking until some discussion has taken place. |
@pmcelhaney well said, and all really good points. I'm not sure I agree that an issue should be closed as soon as a PR related to it is opened, as I think there's value in having the PR merge trigger the issue being closed. For everyone, I've removed the part about requiring a PR for new rules since that's the thing people seem to be getting caught up on and it's not overly important to the proposal. Let's just say new rules can come in as a PR or issue. Okay, I don't like doing this very often but this is a big change that requires the whole team to be on board, so @eslint/eslint-team please review and comment if you haven't already done so. |
Every thing sounds good to me.
I think we should require issue of any core changes so that we can have discussion before making changes. |
Okay, lots of thumbs up on this, so I'm going to assume there are no significant objections, thus marking as accepted. I'll update the documentation when I have energy and pass that around for final approval before we officially implement this. |
tl;dr I think it's possible that we can now accept more PRs without issues. It will take a bit of work, but I think it would streamline our triaging process.
Background
From the beginning, we've required issues for all pull requests outside of documentation changes. This followed a pattern that I enjoyed at work: where the issue represented the problem and the pull request represented the solution. Some of the benefits I saw in doing this from the onset:
Problems
I think this approach served us well at the beginning of the project, but now that we have scaled out a lot, I'm starting to see problems:
What's Changed?
Anyone who's discussed longstanding practices with me knows that my position is always that we don't revisit past decisions unless something has changed. So, to keep myself honest, here are the things that have changed that I think allow us to make further changes:
Proposal
Given where we are, I think we can start accepting PRs without issues in some cases. Here's what I have in mind:
I'd like to see us require PRs for new rule proposals rather than having people request rules through issues.They'd still need to document their proposal in the PR,but any issues that are opened for rule requests we can just close and ask them to open a PR.Things that I think should still require an issue:
Obviously, we'd have to update our documentation and PR/issue templates to reflect these changes. That also might require contributors to copy-paste a form into the PR after they've submitted it, but I think that's easily enforceable and a small price to pay to avoid the overhead of opening a new issue.
Pros
People understand that they must implement new rules themselves in order for the rule to be considered for the core.Cons
We could miss out on good new rule ideas because people can't or won't implement them.Thoughts?
Edited: Removed the part about requiring PRs for rules as that seems to be the most controversial part of this proposal and isn't required to move forward.
The text was updated successfully, but these errors were encountered: