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] Add closing message workflow #43450

Merged

Conversation

michelengelen
Copy link
Member

@michelengelen michelengelen commented Aug 26, 2024

Adds the reusable workflow of adding a message on closed issues (like in thew x repo)

Screenshot 2024-08-26 at 16 26 06

This is using a reusable workflow to decrease friction on changes.

@michelengelen michelengelen added enhancement This is not a bug, nor a new feature scope: infra Org infrastructure work going on behind the scenes labels Aug 26, 2024
@michelengelen michelengelen self-assigned this Aug 26, 2024
@mui-bot
Copy link

mui-bot commented Aug 26, 2024

Netlify deploy preview

https://deploy-preview-43450--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 0dde9b5

@mnajdova
Copy link
Member

Is this comment left only when the issue is resolved/fixed?

@michelengelen
Copy link
Member Author

Is this comment left only when the issue is resolved/fixed?

It's always left ... you don't seem to use the stale issue workflow, right?
In that case we would skip that message, since it gets closed with a different closingReason

@michelengelen
Copy link
Member Author

we could probably adjust this to not post the comment, when the issue is closed as won't do ... i'll check that ... but this would mean just a tiny adjustment in this workflow file

@michelengelen
Copy link
Member Author

@mnajdova b67700b

@mnajdova
Copy link
Member

I meant we likely shouldn't leave it if for example if it has the labels: "status: waiting for author", "duplicated", "support: StackOverflow" etc. We are leaving different messages in these scenarios.

@mnajdova
Copy link
Member

We need to check how are the issues closed in these scenarios (the manual one should be already covered by your last commit).

@michelengelen
Copy link
Member Author

OK, I just looked over the workflows in this repo and it seems you close for a few reasons:

  1. Inactivity: Same as with X, so the behavior is the same => no closing message
  2. Stackoverflow support: This closes the issue and would potentially add the closing message. This change 8382256 fixes that
  3. Marked as duplicate: This is by default marked as 'not_planned' by the action, so we are good there as well.

I could not find any other scenarios, so considering these and the manual action is also covered we should be good with these changes, right? 😄

@@ -32,4 +32,5 @@ jobs:
If you have a question on Stack Overflow, you are welcome to link to it here, it might help others.
If your issue is subsequently confirmed as a bug, and the report follows the issue template, it can be reopened.
close-issue: true
issue-close-reason: 'not planned'
Copy link
Member

@mnajdova mnajdova Aug 26, 2024

Choose a reason for hiding this comment

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

This is what needed to be checked, nice!

Copy link
Member

@oliviertassinari oliviertassinari Aug 26, 2024

Choose a reason for hiding this comment

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

We should roll this change to all the other repositories, right? I imagine it's about https://github.blog/changelog/2022-05-19-the-new-github-issues-may-19th-update/.

I'm also not sure how to feel about not asking for support feedback on an issue we closed as not planned. I guess it makes sense, what we would get is mostly people not happy 😄. Kind of https://www.oesterlund.dev/blog/learnings-from-one-year-of-building-an-open-source-project#learning-2-dont-even-try-to-please-everyone, built a system that makes us default to not try to please everyone.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the plan is to roll this out to all repos that have at least some user visibility.

Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Cool, let's try this! Thanks 🙏

@michelengelen michelengelen enabled auto-merge (squash) August 26, 2024 15:20
@mnajdova
Copy link
Member

I created #43451 to fix the CI issue, @siriwatknp is handling the other changes needed for the stable release

@mnajdova
Copy link
Member

@michelengelen you can update to latest master and merge, the CI blocker was just merged.

@michelengelen michelengelen merged commit 3226205 into mui:master Aug 26, 2024
18 checks passed
aarongarciah pushed a commit to aarongarciah/material-ui that referenced this pull request Aug 26, 2024
@michelengelen michelengelen deleted the automation/add-closing-message branch August 27, 2024 06:44
@aarongarciah
Copy link
Member

@michelengelen I started receiving these email notifications, is there a way to avoid them?

Screenshot 2024-08-27 at 11 56 41

@michelengelen
Copy link
Member Author

@michelengelen I started receiving these email notifications, is there a way to avoid them?

Screenshot 2024-08-27 at 11 56 41

You should just unsubscribe from the workflow runs... you can do that in the GitHub settings.

@aarongarciah
Copy link
Member

@michelengelen yeah, I can unsubscribe from workflow runs notifications, but I like to be notified about failed workflows. I guess the question is: why is this particular workflow failing? These notifications can be very noisy as I think I'm receiving one for every commit pushed to any given PR.

@michelengelen
Copy link
Member Author

@michelengelen yeah, I can unsubscribe from workflow runs notifications, but I like to be notified about failed workflows. I guess the question is: why is this particular workflow failing? These notifications can be very noisy as I think I'm receiving one for every commit pushed to any given PR.

With #43464 this should not fail anymore! 👍🏼

I thought you can unsubscribe from specific workflows as well, but that might not be the case!

@aarongarciah
Copy link
Member

Awesome, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is not a bug, nor a new feature scope: infra Org infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants