Proposal: Leave issues/PRs open for a set amount of time

[nzakas]

The team is fairly large and distributed at this point, with people from a lot of different time zones all over the world (United States, Japan, Russia, Turkey...probably more?). Given that, and the different rates at which people can get online to review issues and PRs, I'd like to propose that we adopt the system used by Node.js, which is basically:

1. PRs should remain open for at least 48 hours before merging (72 hours if over the weekend)
2. Issues should remain open for at least 48 hours before closing (72 hours if over the weekend) - this refers to closing because we aren't going to address the issue. Closing for other reasons (because it's a duplicate, because it's a question that's been answered, etc.) wouldn't need to wait.

The goal is to give more members of the team the opportunity to give feedback on PRs and to ensure issues aren't closed prematurely before people have been able to chime in.

We can always make exceptions as necessary, but those should be reserved for high-priority situations (such as converting every rule to a new format, where waiting will create a lot of merge conflicts for other PRs).

Thoughts?

[ilyavolodin]

While I'm perfectly fine with the above suggestion, I think we need tooling to support those rules. Right now we have A LOT of stuff we need to remember before making a decision. Take PRs for example: We need to make sure that linked issue is accepted and all questions were resolved, title is formatted in the right way, documentation has been updated, if any other team member commented on the issue, that they also left "LGTM" after the update, we haven't release minor version in the last three days, we are not about to release, CLA has been signed, builds are passing, etc. In all honestly, it's too much to remember when you only have a few minutes in the morning to look over PRs.
Also, is it 48 hours since PR has been opened or since last update has been pushed?

[nzakas]

Also, is it 48 hours since PR has been opened or since last update has been pushed?

Since it was opened. If we waited for every change, nothing would get merged.

So basically, you should look for "opened 2 days ago" before merging.

We can definitely look at automating this somehow - I'm just not really sure how to do that. But I wouldn't want that to be a blocker for making this decision. We should put the change into place and then see if anyone has an idea about how to automate it.

[ilyavolodin]

Agree. It shouldn't be a blocker. And as I said before, I'm for the proposed change.

[ilyavolodin]

After trying it for a few day, I really don't think we should do this. What I noticed is that if a PR doesn't get merged on the first day, it tends to hang around for a while. People forget to check it and then we have messages like "Complete lost track of this, sorry.". Same with issues. While I see the reason behind this suggestion, I think it will do more harm then good. We rarely have situations where PR gets merged in and then somebody jumps in and say that it shouldn't have.

[nzakas]

I'm not sure I agree with your assessment. What do you mean by "getting lost"? Can you share some examples?

@eslint/eslint-team other thoughts on this?

[michaelficarra]

It'd be just as easy to share examples of PRs being merged with significant issues because not enough people had time to do a review.

[nzakas]

@michaelficarra that's not the point here. The goal is not to reduce PRs with significant issues, as I said in the original description:

The goal is to give more members of the team the opportunity to give feedback on PRs and to ensure issues aren't closed prematurely before people have been able to chime in.

Since @ilyavolodin has said that achieving this goal is causing other problems, so I think it's fair to ask for examples of those other problems as a data point in this discussion.

[ilyavolodin]

@nzakas Those examples are not easy to come up with after the fact. But, I would've merged #6091 already, but I didn't. I'm sure it will get merged in at some point. You just can't predict when that point is going to be.
I'm going off of my personal workflow here (so it might not apply to anyone else). I usually browse issues/PRs based on the Github notifications. If I left "LGTM" on the PR and nobody added anything else, I will not look at that PR for a long while. I only backtrack PRs when it's a really slow day (under 10 updates) and I have extra time.

[mikesherov]

@nzakas @michaelficarra was agreeing with you.

Let's keep in mind that we can automate whatever policy we have.

ESLint bot could be made to ping a PR 72 hours after it was open if the concern is them getting lost.

Can I add an amendment to this suggestion that if a maintainer responds to an issue with no feedback from OP for two weeks that issues are autoclosed by our bot? This was tremendously helpful for jquery and jqueryui keeping issues moving along and manageable.

[BYK]

@ilyavolodin's concern is a valid one. Although I support this change, I think we should try to find a good solution to those PRs.

One option may be simply one of us taking the patch over and reinstating it if the original author does not respond/comply after a while.

[alberto]

What if we also add a label to those PR we think are ready? It would make it easier to track them for everyone. And if we wanted, we could even automate merging them after the specified timeframe.

[Mouvedia]

Can I add an amendment to this suggestion that if a maintainer responds to an issue with no feedback from OP for two weeks that issues are autoclosed by our bot?

-1 the bot won't be able to tell the difference. And that is something I always hated on the jQuery issue tracker: completely valid and still relevant issues being closed because they were too old.

[mikesherov]

The bot could tell the difference very easily. And it was only when the issue went truly dead. Relevant issues would not be closed too early.

We would be able to say @eslintbot pending in a comment, and if anyone responds at all, don't close.

Point being there are infinite ways to make this work functionally if we like the idea.

[nzakas]

@ilyavolodin i think we all need a better PR workflow. If the workflow is "merge immediately or ignore," that's not really sustainable. What I do is I use notifications first and then, once or twice a week, I scan all open PRs to see what their status is so nothing gets forgotten.

@alberto it's hard to label something as ready, because if it's ready, why wasn't it merged? AMD what if someone finds a problem with it after it was labeled as ready? I think we'd actually end up with more PRs getting lost this way because people would skip the "ready" ones thinking we are done.

Just to refocus a bit: do people generally like the idea of the waiting period? If yes, please add a 👍 reaction to this comment. If you're a "yes but I'm concerned about x", that's still a 👍 For this purpose. If we are all in agreement about the direction then we can work on solutions to various concerns.

[mikesherov]

👍

[pedrottimark]

👍 it is good for us to follow this relevant guideline from Node.js, and also sets a good example by us.

[BYK]

👍

[nzakas]

Okay, it seems we have a consensus on waiting being a good idea. Let's dig into some specifics point-by-point.

I think waiting 48/72 hours before closing an issue is fairly uncontroversial, but let's start there. Any concerns about this for issues only? My thought is that this would only really apply to enhancement and feature requests. Bugs should be fixed as soon as possible, questions can be closed when answered, etc.

Thoughts on that?

[alberto]

Do you mean for closing rejected ones?

Otherwise I don't know how we can take them separately from PRs.

[nzakas]

@alberto yes, I mean closing issues we aren't going to do.

[nzakas]

Okay, to try to move this along, here's a complete proposal. Please provide your thoughts or a 👍 if you think everything is appropriate.

---

Terminology

• Waiting period - two days when the issue or pull request was opened on a weekday; three days when the issue or pull request was opened on a weekend.

Issues

You can close an issue immediately if it:

1. Is a duplicate of an existing issue.
2. Is a question that has been answered completely.

Otherwise, observe the waiting period before closing an unaccepted issue.

Pull Requests

You can merge a pull request immediately if it:

1. Contains small documentation changes (fixing typos, clarifying a sentence, etc.)
2. Is a chore
3. Is build-related and fixes a current build problem (to unblock others)

Otherwise, observe the waiting period before merging.

[btmills]

👍

Just a minor typo, Pull Requests > 3. Is built build-related...