dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.35k stars 4.74k forks source link

Updated issue and PR workflows #44775

Closed danmoseley closed 3 years ago

danmoseley commented 3 years ago

We have a relatively large number of issues and open PR's in this repo, and are considering adding some workflow to help manage them, based on similar workflow that's been successful in the aspnetcore and winforms repos:

  1. When "waiting author feedback” is set on an open issue or PR, and there is no comment (or push) from the author within 14 days, a bot adds "no recent activity" label and a note. After a further 7 days without comment from the author the bot closes the issue or PR. If during any of this time there’s a comment from the author, the labels are cleared and it sets “needs attention” for us.

  2. When an issue has been closed for 30 days without activity, lock it with a note recommending anyone wishing to comment open a new issue. This is because it’s easy to overlook comments on closed issues, and often those comments aren't relevant. Note this would apply to most of our existing closed issues.

Our goal is to help avoid stale issues, and help us be more responsive. We're working on some other ideas as well. We’d welcome your feedback on this plan: we expect to enable it in about a week. In particular are 14 and 7 days the right amounts of time?

[edit: I believe the existing "needs more info" and "needs repro" labels would roll into "waiting author feedback" -- feedback on this welcome.] [edit: the lock after being closed 30 days and without activity would apply to PR's also]

cc @terrajobst @davidwrighton

Dotnet-GitSync-Bot commented 3 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

SingleAccretion commented 3 years ago

There are a lot of issues opened in this repo that will forever be in a limbo until there is a decision that the relevant area needs some investment. Similarly, there are API suggestions that haven't really gathered enough attention to be considered api-ready-for-review, nor have they been outright rejected.

There are bugs that have been open for a quite some time but haven't yet bubbled up the priority ladder/haven't found a contributor willing to fix them. Most of them are somewhat hard problems, not fixable "in an afternoon", some require design considerations, some are stale of course (I myself found two reflection bugs that had been resolved when looking at contributing this summer). I think it is unrealistic to expect all bugs to be fixed in 30 days after they've been discovered and they should automatically be excluded from this system.

Obviously, api-approved issues should be excluded from it as well.

The proposed model works great for "current" issues, slam dunk API proposals - things that are actionable in that 1 month window. But it does sure feel like too big a hammer considering how diverse and rich the set of problems that dotnet/runtime faces is.

Maybe we should consider adding some "Too big/too ambivalent to close" label? I don't know. Maybe, it would be good to effectively "reboot" the repo (like csharplang was rebooted with discussions and community ambassadors) - which what the current proposal is effectively is trying to do, unless I am misunderstanding it. I suspect that'll put a lot of strain on the maintainers in the short term as they would need to triage issues that may or may not be relevant anymore and go as far back as 2015.

terrajobst commented 3 years ago

@SingleAccretion

I think you're confusing what this proposal is about: it's about two scenarios:

  1. Issues explicitly marked as needing input from the author. This excludes issues marked as up-for-grabs, api-approved, untriaged etc.
  2. Issues that are already closed.

Does this make sense?

GSPP commented 3 years ago

Locking an issue would prevent customers from adding "their story". This could entail feedback, a use-case, a proposed solution, a "this is affecting me, too", or a case for reopening. Opening a new issue would be too heavy for these kinds of things. It would not be done. I wonder if locking issues would stifle too much informal conversation.

Maybe there needs to be differentiation between one-off issues (such as bug reports affecting just one person), and issues useful for everyone (feature requests). For the latter group, shutting those down would indeed stiffle a lot. For example, https://github.com/dotnet/runtime/issues/27449 (chunking LINQ method) has been lingering forever yet it has good chances of being done eventually. I don't think we'd want to shut this down.

This issue (https://github.com/dotnet/runtime/issues/17590, static hashing API) took 4 years from feature request to implementation. Such proposals should be allowed to stay alive.

SingleAccretion commented 3 years ago

I think Immo said that only issues tagged with something like needs more info would be subject to this system.

danmoseley commented 3 years ago

Locking an issue would prevent customers from adding "their story". This could entail feedback, a use-case, a proposed solution, a "this is affecting me, too", or a case for reopening. Opening a new issue would be too heavy for these kinds of things. It would not be done. I wonder if locking issues would stifle too much informal conversation

We want to optimize for not missing comments like "this didn't fully fix the problem". Currently it's a bit random whether we see those because our workflow is built around open issues. Even comments like "please reconsider" are probably best handled in a new issue because then it goes into our list to triage, and perhaps a new issue can be more focused than reopening the old issue.

This issue (#17590, static hashing API) took 4 years from feature request to implementation. Such proposals should be allowed to stay alive.

This proposal does not relate to closing issues that are currently open.

PathogenDavid commented 3 years ago

1 is probably fine, but for 2: In general I don't think I've ever participated or explored in a repository that automatically locks issues and not had it feel community-hostile.

I strongly agree with @GSPP here. In the context of an API proposal (for example), not everyone is going to have enough care/time/bravery to open a new proposal, but they might have something to add that someone else who does care can take into account when opening a new proposal.

Locking the issue also ignores community members subscribed to closed issues. If a new issue is opened, subscribed users have no way to be notified that the new issue was opened. I know that if I revived a dead API proposal, I'd want to be able to comment on the old issue to notify any subscribers that I've revived it.

Maybe the issue reviver has the consideration to mention people who commented on the original thread, but that ignores people who subscribed without commenting. Maybe I'm the weird one here, but according to GitHub I'm subscribed to 129 different issues on the runtime repo even though I've only actually commented on/authored 23 issues. I've maybe added a 👍 on most of them, but otherwise there's not even a public record that I care about them.

There are also occasionally scenarios where someone finds a workaround or builds an alternative solution to a problem and wants to share their solution. In this scenario, it makes 0 sense to open a new issue because commenting your solution only really serves to benefit the people who were subscribed to the dead issue.

For example: https://github.com/dotnet/runtime/issues/4882 is never going to get official runtime support. Its replacement only covers sane subset of the features it provided. However, despite it not being a good idea there are ways to accomplish similar functionality without help from the runtime. They're platform-specific and not super fun though, so if someone got everything figured out and made a tidy NuGet package to consume it, I think non-runtime people watching the issue would probably appreciate knowing.

Another example is https://github.com/dotnet/runtime/issues/4121 It's closed to signal it's never going to happen as far as the runtime team is concerned. However, if any masochists out there made a fork of Clang to add C++/CLI support, commenting here would be a good way to know interested parties it now exists.

(Sorry for the hypotheticals. I know I've benefitted from these sorts of comments before, but I can't think of a specific instance. Although you could argue the C++/CLI one is a good example since there's lots of alternative solutions mentioned in there, albeit they happened before it was closed recently.)


We want to optimize for not missing comments like "this didn't fully fix the problem". Currently it's a bit random whether we see those because our workflow is built around open issues. Even comments like "please reconsider" are probably best handled in a new issue because then it goes into our list to triage, and perhaps a new issue can be more focused than reopening the old issue.

I do agree these situations are an issue. I think a better solution would be to have the bot edit the issue to add a prominent, resolution-specific notice to the very top of the issue encouraging people to open a new one. For example, a closed-as-fixed bug might have a notice like this:

⚠ Closed issue notice ⚠

This issue has been closed and has not received any activity for over 30 days.

If you're still experiencing the problems described by this issue, please open a new issue instead of commenting as this one is no longer being monitored by the runtime team.

Source of that issue notice ```html
⚠ Closed issue notice ⚠

This issue has been closed as fixed and has not received any activity for over 30 days.

If you're still experiencing the problems described by this issue, please open a new issue instead of commenting as this one is no longer being monitored by the runtime team.

```

If you want to see what it looks like on an actual issue, see here: https://github.com/PathogenPlayground/GitHubPlayground2/issues/1

The wording would vary depending on the reason the issue was closed. Another example for an API proposal that was closed as rejected:

⚠ Closed issue notice ⚠

This issue has been closed and has not received any activity for over 30 days.

This API proposal was rejected. If you wish to request a similar feature, please open a new issue instead of commenting as this one is no longer being monitored by the runtime team.

I don't think leaving the same message as a comment on the issue would have the same effect because people don't always read anything besides the first sentence of the original issue. (And I think this is why some large open source projects tend to gravitate towards locking old issues because they configure bots to just leave similar notices as comments and that inevitably fails.)

Anyway that got pretty long, thanks for the consideration if you read to this point 💜

janvorli commented 3 years ago

I've found comments from customers on closed issues useful many times in the past. Making comment on such an issue also made me refresh my memory all the related problems easily. It feels like locking such issues might cause unnecessary friction for people to add a new issue.

vcsjones commented 3 years ago

Most of what I've read makes sense in the context of issues, but I'm a little fuzzy on the mechanics of this for pull requests.

When "waiting author feedback” is set on an open issue or PR, and there is no comment (or push) from the author within 14 days, a bot adds "no recent activity" label and a note. After a further 7 days without comment from the author the bot closes the issue or PR.

and further:

When an issue has been closed for 30 days without activity

Though you only called out issues here, just to clarify, would PRs get locked? I'm pretty sure it's taken me more than 30 days to respond to some PR feedback (🦥), locking a PR would probably mean folks open a new PR, which loses review context.

If it doesn't apply to PRs, great. If it does, I wonder if the 14/21/30 day periods make sense. Or, alternatively, an understood path someone can take to say "I need more more time but I have not forgotten!"

On the topic of PRs, I would consider locking merged PRs some number of days after being merged.

Seems like a solid plan, though I would encourage another issue like this 30 days after implementation to gather feedback and adjust, if needed.

danmoseley commented 3 years ago

Though you only called out issues here, just to clarify, would PRs get locked? I'm pretty sure it's taken me more than 30 days to respond to some PR feedback (🦥), locking a PR would probably mean folks open a new PR, which loses review context.

Yes, PR's also. I'll clarify above. Note that using the (arbitrarily picked) numbers above, it would be 7 + 14 + 30 days from the PR being labeled to it being locked.

I would encourage another issue like this 30 days after implementation to gather feedback and adjust, if needed.

Good idea.

danmoseley commented 3 years ago

OK, thanks for the feedback.

For (1) we will go ahead with this workflow for just issues right now. We will use the existing "needs more info" label (and retire the "needs repo label")

For (2) we will implement as described above for both issues and PRs.

danmoseley commented 3 years ago

I'll leave this active until it's done.

danmoseley commented 3 years ago

Also need to update project docs.

danmoseley commented 3 years ago

For (1) we will go ahead with this workflow for just issues right now. We will use the existing "needs more info" label (and retire the "needs repo label")

Turns out "needs more info" was overloaded. I reversed ~60 edits that triggered. I instead created "needs author feedback"

danmoseley commented 3 years ago

This is now implemented. The locking is taking a while to catch up.