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.

Sign up for GitHub

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

Jump to bottom

tools: add a at here tag for slack messages and update variable for default branch #47358

Closed
wants to merge 2 commits into from
Closed

tools: add a at here tag for slack messages and update variable for default branch #47358

wants to merge 2 commits into from

Conversation

debadree25
Copy link
Member

Adds a @here to the slack message to notify anyone online

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/actions

@debadree25 debadree25 requested a review from aduh95 April 1, 2023 07:12
@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Apr 1, 2023
@debadree25 debadree25 requested a review from Trott April 1, 2023 07:13
aduh95
aduh95 reviewed Apr 1, 2023
View reviewed changes
Copy link
Contributor

@aduh95 aduh95 left a comment

Choose a reason for hiding this comment

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

Looking at the Slack message, it looks like github.repository.default_branch is an empty string, could we also fix that?

@debadree25
Copy link
Member Author

Since the action only runs on the main branch should we just hardcode it? @aduh95

@aduh95
Copy link
Contributor

aduh95 commented Apr 1, 2023

I think we should use github.ref that would make more sense here.

@debadree25
Copy link
Member Author

Seems like github.ref_name is the variable to use ref: https://docs.github.com/en/actions/learn-github-actions/contexts#github-context

@debadree25 debadree25 changed the title tools: add a @here tag for slack messages tools: add a at here tag for slack messages Apr 1, 2023
@debadree25 debadree25 requested a review from aduh95 April 1, 2023 09:48
@debadree25 debadree25 changed the title tools: add a at here tag for slack messages tools: add a at here tag for slack messages and update variable for default branch Apr 1, 2023
@debadree25 debadree25 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Apr 1, 2023
@debadree25
Copy link
Member Author

debadree25 commented Apr 1, 2023 •
edited
Loading

added rebase tag to land as two separate logic changes one for the @/here and another for the refname

Trott
Trott approved these changes Apr 2, 2023
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

LGTM. I proposed this so I'm definitely in favor. Thanks for doing it! There are over 500 people in that Slack channel and there may be some concern about this being noisy, but I'd prefer we try it and make a determination about it. These kinds of things are exactly the kinds of things I want to be highlighted in that channel, so a notification seems helpful. The other option is to move it to a separate channel, but that risks fragmentation--too many people not bothering to join the alerts channel because they are already in the core channel--that kind of thing.

@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 3, 2023
@nodejs-github-bot
Copy link
Collaborator

Landed in 5db08cf...81bb1b0

nodejs-github-bot pushed a commit that referenced this pull request Apr 3, 2023
@debadree25 @nodejs-github-bot
tools: add a at here tag for slack messages
1265eae 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
nodejs-github-bot pushed a commit that referenced this pull request Apr 3, 2023
@debadree25 @nodejs-github-bot
tools: use ref_name to get branch pushed on
81bb1b0 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott

This comment was marked as resolved.

Sign in to view
@Trott
Copy link
Member

Trott commented Apr 3, 2023

No big deal, but I think this should have used the squash label rather than the rebase label. Not sure if it was a mistake or a difference in landing philosophy.

Eh, nah, looking more closely, it could definitely be rebased. I'm wrong. (Or at least, both approaches are valid here.)

@debadree25
Copy link
Member Author

debadree25 commented Apr 3, 2023 •
edited
Loading

yeah since one changes the message another fixes the bug where the name branch is not appearing hence landing as two. 😅

RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@debadree25 @RafaelGSS
tools: add a at here tag for slack messages
ecc5178 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@debadree25 @RafaelGSS
tools: use ref_name to get branch pushed on
6c3fdb6 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@debadree25 @RafaelGSS
tools: add a at here tag for slack messages
b9e6584 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@debadree25 @RafaelGSS
tools: use ref_name to get branch pushed on
0fd4793 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
@debadree25 @RafaelGSS
tools: add a at here tag for slack messages
7bddf7b 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
@debadree25 @RafaelGSS
tools: use ref_name to get branch pushed on
1eb15b7 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
@debadree25 @RafaelGSS
tools: add a at here tag for slack messages
59a9ce3 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
@debadree25 @RafaelGSS
tools: use ref_name to get branch pushed on
2febad3 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 8, 2023
@debadree25 @RafaelGSS
tools: add a at here tag for slack messages
b1d6a15 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 8, 2023
@debadree25 @RafaelGSS
tools: use ref_name to get branch pushed on
9c975f7 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
@debadree25 @danielleadams
tools: add a at here tag for slack messages
59a71f0 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
@debadree25 @danielleadams
tools: use ref_name to get branch pushed on
9801c15 
PR-URL: #47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
@debadree25 @MoLow
tools: add a at here tag for slack messages
28935a8 
PR-URL: nodejs#47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
MoLow pushed a commit to MoLow/node that referenced this pull request Jul 6, 2023
@debadree25 @MoLow
tools: use ref_name to get branch pushed on
35e6cf2 
PR-URL: nodejs#47358
Reviewed-By: Antoine du Hamel <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@danielleadams danielleadams mentioned this pull request Jul 10, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@richardlau richardlau richardlau approved these changes

@targos targos targos approved these changes

@Trott Trott Trott approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. meta Issues and PRs related to the general management of the project.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@debadree25 @nodejs-github-bot @aduh95 @Trott @targos @richardlau