-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
ci: update of files from global .github repo #4266
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
Conversation
✅ Deploy Preview for asyncapi-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThe workflow file for notifying TSC members was updated to use an environment variable Changes
Estimated code review effort1 (~2 minutes) Suggested labels
Suggested reviewers
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4266--asyncapi-website.netlify.app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/notify-tsc-members-mention.yml(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: akshatnema
PR: asyncapi/website#3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:41:29.632Z
Learning: When updating workflows for the AsyncAPI website repository, use `.github/workflows/if-nodejs-pr-testing.yml` to include environment variables and secrets for Node.js PR testing.
Learnt from: anshgoyalevil
PR: asyncapi/website#3950
File: scripts/utils/check-locales.ts:122-129
Timestamp: 2025-04-20T16:05:16.482Z
Learning: In the AsyncAPI website project, Next.js throws errors at runtime when locale files are missing, making additional validation for missing files unnecessary in the check-locales script.
Learnt from: asyncapi-bot
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
- `/please-take-a-look` or `/ptal`: Requests attention from reviewers who haven't reviewed the PR
- `/ready-to-merge` or `/rtm`: Triggers automerge when all conditions are met
- `/do-not-merge` or `/dnm`: Blocks automerge even if all conditions are met
- `/autoupdate` or `/au`: Adds autoupdate label to keep PR in sync with target branch
- `/update` or `/u`: One-time update of PR with latest changes from target branch
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: tests/fixtures/markdown/check-edit-links-data.js:3-11
Timestamp: 2025-01-19T04:51:41.255Z
Learning: In the AsyncAPI website repository, the test data in `tests/fixtures/markdown/check-edit-links-data.js` intentionally includes inconsistent paths (with and without 'docs' prefix) to verify the script's ability to normalize and handle ambiguous path structures.
Learnt from: akshatnema
PR: asyncapi/website#3265
File: tests/fixtures/toolsObjectData.js:51-52
Timestamp: 2024-10-09T17:35:36.557Z
Learning: When reviewing code in the 'asyncapi/website' repository, akshatnema prefers that I do not provide committable code suggestions.
Learnt from: akshatnema
PR: asyncapi/website#3423
File: tests/index.test.js:2-7
Timestamp: 2025-01-18T08:44:43.614Z
Learning: In the AsyncAPI website project, JavaScript test files must include the .ts extension when importing TypeScript files (e.g., `require('../scripts/build-rss.ts')`). This is a project-specific requirement enforced by the linting rules and build setup, even though it differs from typical Node.js behavior.
Learnt from: anshgoyalevil
PR: asyncapi/website#3557
File: scripts/markdown/check-editlinks.js:58-59
Timestamp: 2025-01-08T15:15:00.759Z
Learning: In the AsyncAPI codebase, batch processing operations (like in the Dashboard script and check-editlinks.js) follow a sequential pattern using await in loops, which is the preferred approach for maintaining consistency across the codebase.
.github/workflows/notify-tsc-members-mention.yml (2)
Learnt from: akshatnema
PR: #3378
File: scripts/markdown/check-markdown.js:1-1
Timestamp: 2024-11-25T18:41:29.632Z
Learning: When updating workflows for the AsyncAPI website repository, use .github/workflows/if-nodejs-pr-testing.yml to include environment variables and secrets for Node.js PR testing.
Learnt from: asyncapi-bot
PR: asyncapi/website#0
File: :0-0
Timestamp: 2025-02-18T12:07:42.211Z
Learning: The following PR commands are supported in the asyncapi/website repository:
/please-take-a-lookor/ptal: Requests attention from reviewers who haven't reviewed the PR/ready-to-mergeor/rtm: Triggers automerge when all conditions are met/do-not-mergeor/dnm: Blocks automerge even if all conditions are met/autoupdateor/au: Adds autoupdate label to keep PR in sync with target branch/updateor/u: One-time update of PR with latest changes from target branch
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: Automerge PR autoapproved by a bot
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
| TITLE: ${{github.event.issue.title}} | ||
| with: | ||
| script: | | ||
| const sendEmail = require('./.github/workflows/scripts/mailchimp/index.js'); | ||
| sendEmail('${{github.event.issue.html_url}}', '${{github.event.issue.title}}'); | ||
| sendEmail('${{github.event.issue.html_url}}', '$TITLE'); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$TITLE is passed as a literal string — use process.env.TITLE instead
Inside the github-script step the JS code runs in a Node context, not a POSIX shell, therefore the $TITLE placeholder is not expanded.
sendEmail will currently receive the literal text "$TITLE" rather than the issue title.
- TITLE: ${{github.event.issue.title}}
...
- sendEmail('${{github.event.issue.html_url}}', '$TITLE');
+ TITLE: ${{ github.event.issue.title }}
...
+ sendEmail('${{github.event.issue.html_url}}', process.env.TITLE);The same fix is required for every job in this workflow where TITLE is introduced.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| TITLE: ${{github.event.issue.title}} | |
| with: | |
| script: | | |
| const sendEmail = require('./.github/workflows/scripts/mailchimp/index.js'); | |
| sendEmail('${{github.event.issue.html_url}}', '${{github.event.issue.title}}'); | |
| sendEmail('${{github.event.issue.html_url}}', '$TITLE'); | |
| TITLE: ${{ github.event.issue.title }} | |
| with: | |
| script: | | |
| const sendEmail = require('./.github/workflows/scripts/mailchimp/index.js'); | |
| sendEmail('${{github.event.issue.html_url}}', process.env.TITLE); |
🤖 Prompt for AI Agents
In .github/workflows/notify-tsc-members-mention.yml around lines 69 to 74, the
string '$TITLE' is passed literally to the sendEmail function instead of the
actual issue title because environment variables are not expanded in the Node.js
context. Replace '$TITLE' with process.env.TITLE to correctly pass the issue
title. Apply this fix to all occurrences where TITLE is used in the workflow.
| TITLE: ${{github.event.pull_request.title}} | ||
| with: | ||
| script: | | ||
| const sendEmail = require('./.github/workflows/scripts/mailchimp/index.js'); | ||
| sendEmail('${{github.event.pull_request.html_url}}', '${{github.event.pull_request.title}}'); | ||
| sendEmail('${{github.event.pull_request.html_url}}', '$TITLE'); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same interpolation problem for PR titles
$TITLE is not expanded in Node. Replace with process.env.TITLE as shown above.
🤖 Prompt for AI Agents
In .github/workflows/notify-tsc-members-mention.yml around lines 115 to 120, the
variable $TITLE is used inside the Node script but it is not expanded correctly.
Replace '$TITLE' with process.env.TITLE to access the environment variable
properly within the Node script, ensuring the PR title is passed correctly.
| TITLE: ${{github.event.discussion.title}} | ||
| with: | ||
| script: | | ||
| const sendEmail = require('./.github/workflows/scripts/mailchimp/index.js'); | ||
| sendEmail('${{github.event.discussion.html_url}}', '${{github.event.discussion.title}}'); | ||
| sendEmail('${{github.event.discussion.html_url}}', '$TITLE'); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussion job: $TITLE remains a literal
Apply the process.env.TITLE fix here as well.
🤖 Prompt for AI Agents
In .github/workflows/notify-tsc-members-mention.yml around lines 161 to 166, the
variable $TITLE is used as a literal string instead of referencing the
environment variable. Update the script to use process.env.TITLE instead of
'$TITLE' so that the actual title value is passed to the sendEmail function.
| TITLE: ${{github.event.issue.title}} | ||
| with: | ||
| script: | | ||
| const sendEmail = require('./.github/workflows/scripts/mailchimp/index.js'); | ||
| sendEmail('${{github.event.comment.html_url}}', '${{github.event.issue.title}}'); | ||
| sendEmail('${{github.event.comment.html_url}}', '$TITLE'); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Issue-comment job: fix title interpolation
Replace $TITLE with process.env.TITLE.
🤖 Prompt for AI Agents
In .github/workflows/notify-tsc-members-mention.yml around lines 207 to 212, the
script incorrectly uses the literal string '$TITLE' instead of referencing the
environment variable. Replace '$TITLE' with 'process.env.TITLE' in the sendEmail
function call to correctly interpolate the title value from the environment.
| TITLE: ${{github.event.issue.title}} | ||
| with: | ||
| script: | | ||
| const sendEmail = require('./.github/workflows/scripts/mailchimp/index.js'); | ||
| sendEmail('${{github.event.comment.html_url}}', '${{github.event.issue.title}}'); | ||
| sendEmail('${{github.event.comment.html_url}}', '$TITLE'); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR-comment job: title not expanded
Use process.env.TITLE when calling sendEmail.
🤖 Prompt for AI Agents
In .github/workflows/notify-tsc-members-mention.yml around lines 253 to 258, the
TITLE variable is not being expanded correctly in the sendEmail call. Replace
the string '$TITLE' with process.env.TITLE to correctly pass the expanded title
environment variable to the sendEmail function.
| TITLE: ${{github.event.discussion.title}} | ||
| with: | ||
| script: | | ||
| const sendEmail = require('./.github/workflows/scripts/mailchimp/index.js'); | ||
| sendEmail('${{github.event.comment.html_url}}', '${{github.event.discussion.title}}'); | ||
| sendEmail('${{github.event.comment.html_url}}', '$TITLE'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussion-comment job: title still a literal
Change to process.env.TITLE for correct expansion.
🤖 Prompt for AI Agents
In .github/workflows/notify-tsc-members-mention.yml around lines 299 to 303, the
TITLE variable is used as a literal string '$TITLE' instead of referencing the
environment variable. Replace '$TITLE' with 'process.env.TITLE' in the sendEmail
function call to correctly expand and use the TITLE value.
|
@thulieblack @derberg Should we accept this PR with above errors, explained by CodeRabbitAi? |
|
@akshatnema yes, this is anyway coming from .github. Coderabbit is not always right |
|
/rtm |
|
/rtm |
TRohit20
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/rtm
|
/rtm |
|
/rtm |
|
actually this time coderabbit was right, closing, there will be another PR |
Summary by CodeRabbit