Skip to content
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

infra: auto comment on feature requests #2041

Merged
merged 12 commits into from
Apr 23, 2023

Conversation

xDivisionByZerox
Copy link
Member

Description

This PR adds a workflow that automatically comments on issues that get the s: waiting for user interest label assigned.

This allows us to reduce maintenance resources on feature requests by first checking whether a feature is actually requested by the community instead of a single individual.

@xDivisionByZerox xDivisionByZerox added p: 1-normal Nothing urgent c: infra Changes to our infrastructure or project setup labels Apr 12, 2023
@xDivisionByZerox xDivisionByZerox added this to the vFuture milestone Apr 12, 2023
@xDivisionByZerox xDivisionByZerox requested a review from a team April 12, 2023 18:49
@xDivisionByZerox xDivisionByZerox self-assigned this Apr 12, 2023
@xDivisionByZerox xDivisionByZerox requested a review from a team as a code owner April 12, 2023 18:49
@codecov
Copy link

codecov bot commented Apr 12, 2023

Codecov Report

Merging #2041 (4cef0cd) into next (8fc5261) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #2041      +/-   ##
==========================================
- Coverage   99.59%   99.58%   -0.01%     
==========================================
  Files        2538     2538              
  Lines      242656   242656              
  Branches     1299     1296       -3     
==========================================
- Hits       241666   241661       -5     
- Misses        963      968       +5     
  Partials       27       27              

see 2 files with indirect coverage changes

Copy link
Member

@ST-DDT ST-DDT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it. Just some minor things.

.github/workflows/comment-issue.yml Outdated Show resolved Hide resolved
.github/workflows/comment-issue.yml Show resolved Hide resolved
@ST-DDT
Copy link
Member

ST-DDT commented Apr 12, 2023

Can you please create an issue template in the test project so that the external person can set the labels?
Because then the user might not have permissions to post the comment on behalf of the repository.

@matthewmayer
Copy link
Contributor

honestly, i don't like it. particularly for a new contributor it seems a bit off-putting to be sent a completely canned response by a robot. if there were hundreds of these suggestions sure but it's like 1 or 2 a month?

@ST-DDT ST-DDT added the s: needs decision Needs team/maintainer decision label Apr 13, 2023
@ST-DDT
Copy link
Member

ST-DDT commented Apr 13, 2023

Can you please create an issue template in the test project so that the external person can set the labels? Because then the user might not have permissions to post the comment on behalf of the repository.

Works as expected

https://github.com/xDivisionByZerox/test-issue-comment-on-label/issues/5#issuecomment-1506455845

@ST-DDT
Copy link
Member

ST-DDT commented Apr 13, 2023

Team Decision

The voting on this issue was ambiguous.
So we are waiting on more input before we decide on this one.

@xDivisionByZerox
Copy link
Member Author

honestly, i don't like it. particularly for a new contributor it seems a bit off-putting to be sent a completely canned response by a robot. if there were hundreds of these suggestions sure but it's like 1 or 2 a month?

@matthewmayer I look at it the other way around.
By automatically replying to an issue that just got the s: waiting for user interest label assigned, the user gets more information on what exactly is happening.
The fact is, every feature request should get this label. We also know that we need to carefully think about which features to support since each additional feature potentially adds a lot of overhead and maintenance. So what this automatic reply does is basically what each of us would do minus we don't need to search for the existing template text.

Please keep in mind this reply ONLY applies if an issue (or maybe PR) gets the s: waiting for user interest label assigned. Yes, this includes every issue opened via the feature report issue template (done in #2028). If you want to work around this you are free to use the freestyle issue.

@pkuczynski
Copy link
Member

For me it is okay. New contributors would get some feedback that we await more interest. We could think of a bit more explanatory and soft message...

@xDivisionByZerox
Copy link
Member Author

We could think of a bit more explanatory and soft message...

Totally agree here. I simply proposed a first message that (at least I thought so) projects what we want to do here.
I'm totally fine going into more detail on:

  • Why we waiting for more user interest
  • How a contributor can help us achieve their request

@damienwebdev
Copy link
Member

I think this is a great idea. One of the most difficult decisions that can be made as a maintainer is whether or not the maintenance burden of a feature outweighs the value it provides to your package consumers.

If we can do anything to align this better, I'm all for it!

@matthewmayer
Copy link
Contributor

Fair enough. I can help soften the message a little!

@ST-DDT
Copy link
Member

ST-DDT commented Apr 14, 2023

Maybe we can write something like this:

Thanks for feature proposal.

We marked it as "waiting for user interest" for now to gather some feedback from our community:

  • If you would like to see this feature being implemented, please react to the description with an up-vote (:+1:).
  • If you have a suggestion or want to point out some special cases that needs to be considered, please leave a comment, so we are aware on them.

We would also like to hear about your use cases for the feature to give us a better understanding of potentially implied or explicit requirements.

We will start the implementation based on:

  • the number of votes and comments
  • the relevance for the ecosystem
  • availability of alternatives and workarounds
  • and the complexity of the requested feature

We do this because:

  • There are plenty of languages/countries out there and we would like to ensure that every method can cover all or almost all of them.
  • Every feature we add to faker has "costs" associated to it:
    • initial costs: design, implementation, reviews, documentation
    • running costs: awareness of the feature itself, more complex module structure, increased bundle size, more work during refactors

@xDivisionByZerox
Copy link
Member Author

For now, I will swap to @ST-DDT's suggestion. We can then discuss single phrases in the code itself.

.github/workflows/comment-issue.yml Outdated Show resolved Hide resolved
.github/workflows/comment-issue.yml Outdated Show resolved Hide resolved
.github/workflows/comment-issue.yml Outdated Show resolved Hide resolved
@matthewmayer
Copy link
Contributor

Could the bot also add a single upvote to get it started? It's easy to locate it if someone has already added a 👍 otherwise you have to hunt in the emoji menu

@xDivisionByZerox
Copy link
Member Author

Could the bot also add a single upvote to get it started? It's easy to locate it if someone has already added a 👍 otherwise you have to hunt in the emoji menu

Yes, it is possible to add reactions to issues via workflow actions. I need to search up how to do that conditionally but should be easy I guess. Good suggestion 👍

Should the bot always vote? Like even if the issue already has been upvoted (altho I thought someone is that fast)? Or only react if no upvote is present?

@ST-DDT
Copy link
Member

ST-DDT commented Apr 14, 2023

Should the bot always vote?

Yes, to keep it "fair".

.github/workflows/comment-issue.yml Outdated Show resolved Hide resolved
.github/workflows/comment-issue.yml Outdated Show resolved Hide resolved
.github/workflows/comment-issue.yml Outdated Show resolved Hide resolved
Co-authored-by: ST-DDT <ST-DDT@gmx.de>
Co-authored-by: Matt Mayer <matt@lemi.travel>
Co-authored-by: Eric Cheng <ericcheng9316@gmail.com>
@xDivisionByZerox xDivisionByZerox removed the s: needs decision Needs team/maintainer decision label Apr 14, 2023
@xDivisionByZerox xDivisionByZerox added the s: accepted Accepted feature / Confirmed bug label Apr 14, 2023
@xDivisionByZerox xDivisionByZerox force-pushed the infra/auto-comment-on-feature-requests branch from 2e265b3 to 66fd797 Compare April 14, 2023 23:46
Co-authored-by: Eric Cheng <ericcheng9316@gmail.com>
import-brain
import-brain previously approved these changes Apr 20, 2023
@xDivisionByZerox xDivisionByZerox requested review from a team April 20, 2023 21:37
ST-DDT
ST-DDT previously approved these changes Apr 21, 2023
.github/workflows/comment-issue.yml Show resolved Hide resolved
@ST-DDT ST-DDT requested review from matthewmayer and a team April 21, 2023 21:11
@matthewmayer
Copy link
Contributor

Maybe we could also add a link to an issue search for all issues labelled "awaiting user interest". If a user adds a new issue it might also be nice to know which other issues they consider useful.

@xDivisionByZerox
Copy link
Member Author

[...] If a user adds a new issue it might also be nice to know which other issues they consider useful.

I like that idea. Not only for the user alone but for general awareness.
I'll suggest a snippet.

@xDivisionByZerox xDivisionByZerox dismissed stale reviews from import-brain and ST-DDT via 2f107b4 April 22, 2023 12:18
@ST-DDT
Copy link
Member

ST-DDT commented Apr 22, 2023

Could you please post a screenshot of the result?

@xDivisionByZerox
Copy link
Member Author

Could you please post a screenshot of the result?

image

@ST-DDT ST-DDT merged commit f795269 into next Apr 23, 2023
@ST-DDT ST-DDT deleted the infra/auto-comment-on-feature-requests branch April 23, 2023 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: infra Changes to our infrastructure or project setup p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants