solves #3778, add featured label and integrated workflow with slack #3786
Conversation
thedeeppp
requested review from
Mayaleeeee,
akshatnema,
anshgoyalevil,
asyncapi-bot-eve and
derberg
as code owners
WalkthroughThis update introduces a new GitHub Actions workflow that sends a Slack notification when commits are pushed to the master branch. Additionally, the BlogPostItem component has been updated to conditionally display a "Featured" label based on the post data, along with a minor styling adjustment. Typographical corrections were made in the markdown about page to enhance textual accuracy. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Slack as Slack API
Dev->>GH: Push commit to master branch
GH->>GH: Trigger slack-notifications workflow
GH->>Slack: Send notification via Slack action
Slack-->>GH: Acknowledge notification
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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:
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
|
|
We require all PRs to follow Conventional Commits specification. |
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
please let me know if further changes needed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/slack-notifications.yml (2)
15-18: Enhanced notification message would provide better context.The current message is very generic. Consider enhancing it with more context about the commit that triggered the workflow.
- SLACK_MESSAGE: 'A workflow has been triggered on the master branch.' + SLACK_MESSAGE: 'New changes pushed to master branch by ${{ github.actor }}. Commit message: "${{ github.event.head_commit.message }}"'🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
[error] 18-18: trailing spaces
(trailing-spaces)
18-18: Fix YAML formatting issues.The file has YAML linting errors including trailing spaces on line 18 and missing newline at the end of the file.
- SLACK_MESSAGE: 'A workflow has been triggered on the master branch.' + SLACK_MESSAGE: 'A workflow has been triggered on the master branch.' +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
[error] 18-18: trailing spaces
(trailing-spaces)
components/navigation/BlogPostItem.tsx (1)
67-71: Use consistent BEM naming for the "Featured" label classes.For better maintainability and consistency with component styling, consider using BEM naming for the featured label classes, and potentially extracting the styles to a dedicated class.
- <span className='featured-label bg-yellow-100 text-yellow-800 font-bold px-2 py-1 rounded-md absolute top-2 left-2 z-10'> + <span className='blog-post-item__featured-label bg-yellow-100 text-yellow-800 font-bold px-2 py-1 rounded-md absolute top-2 left-2 z-10'>🧰 Tools
🪛 ESLint
[error] 68-68: Replace
"featured-label·bg-yellow-100·text-yellow-800·font-bold·px-2·py-1·rounded-md·absolute·top-2·left-2·z-10"with'featured-label·bg-yellow-100·text-yellow-800·font-bold·px-2·py-1·rounded-md·absolute·top-2·left-2·z-10'(prettier/prettier)
[error] 68-68: Unexpected usage of doublequote.
(jsx-quotes)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/slack-notifications.yml(1 hunks)components/navigation/BlogPostItem.tsx(3 hunks)
🧰 Additional context used
🪛 ESLint
components/navigation/BlogPostItem.tsx
[error] 38-38: 'isFeatured' is assigned a value but never used.
(unused-imports/no-unused-vars)
[error] 38-38: 'isFeatured' is assigned a value but never used.
(no-unused-vars)
[error] 68-68: Replace "featured-label·bg-yellow-100·text-yellow-800·font-bold·px-2·py-1·rounded-md·absolute·top-2·left-2·z-10" with 'featured-label·bg-yellow-100·text-yellow-800·font-bold·px-2·py-1·rounded-md·absolute·top-2·left-2·z-10'
(prettier/prettier)
[error] 68-68: Unexpected usage of doublequote.
(jsx-quotes)
🪛 YAMLlint (1.35.1)
.github/workflows/slack-notifications.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
[error] 18-18: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 180000ms (4)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
- GitHub Check: Lighthouse CI
🔇 Additional comments (3)
.github/workflows/slack-notifications.yml (1)
1-18:Details
❓ Verification inconclusive
Ensure proper webhook configuration in repository secrets.
This new workflow looks well-structured for notifying Slack when pushes occur to the master branch. It uses a pinned version of the Slack notification action, which is good practice for stability.
Make sure the
WEBSITE_SLACK_WEBHOOKsecret is properly configured in your repository settings before merging.
🏁 Script executed:
#!/bin/bash # Check if the required secret is set up in the repository gh secret list | grep -i WEBSITE_SLACK_WEBHOOK || echo "Warning: WEBSITE_SLACK_WEBHOOK secret not found"Length of output: 302
Webhook Secret Verification: Please Verify Manually
The workflow itself is well-structured and uses a pinned version of the Slack notification action. However, our automated check returned an HTTP 403 error when attempting to list the secrets (i.e.,
WEBSITE_SLACK_WEBHOOK), which is likely due to the GitHub integration not having sufficient permissions to access this information. As a result, the test script couldn’t confirm if the secret is actually missing.
- Action Item: Manually verify in your repository settings that the
WEBSITE_SLACK_WEBHOOKsecret is properly configured with the correct value.- Note: The permission error in the automated check does not conclusively indicate that the secret is absent—it may simply reflect integration access limitations.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
[error] 18-18: no new line character at the end of file
(new-line-at-end-of-file)
[error] 18-18: trailing spaces
(trailing-spaces)
components/navigation/BlogPostItem.tsx (2)
58-58: Good addition of therelativeclass.Adding the
relativeclass to the list item is necessary to properly position the absolutely positioned "Featured" label. This is a good change that ensures correct positioning of the new element.
67-71:Details
❓ Verification inconclusive
Verify that
post.featuredis properly documented in the interface.Make sure that the
featuredproperty is properly documented in theIBlogPostinterface. It might be worth checking if this is a new property being added to the interface.
🏁 Script executed:
#!/bin/bash # Find the IBlogPost interface definition to verify the featured property rg -A 20 "interface IBlogPost" --type ts # Find any existing usage of the featured property rg "\.featured" --type tsLength of output: 727
Action Required: Verify Documentation for
post.featuredin the IBlogPost InterfaceThe
.featuredproperty is used in multiple components (e.g., in BlogPostItem, NewsroomBlogPosts, NewsroomSection, and blog index files). However, the initial search did not clearly reveal that it is documented within theIBlogPostinterface. Please confirm that thefeaturedproperty is defined and accompanied by proper documentation in theIBlogPostinterface. If this is a newly added property, ensure that both its type declaration and documentation are updated accordingly.🧰 Tools
🪛 ESLint
[error] 68-68: Replace
"featured-label·bg-yellow-100·text-yellow-800·font-bold·px-2·py-1·rounded-md·absolute·top-2·left-2·z-10"with'featured-label·bg-yellow-100·text-yellow-800·font-bold·px-2·py-1·rounded-md·absolute·top-2·left-2·z-10'(prettier/prettier)
[error] 68-68: Unexpected usage of doublequote.
(jsx-quotes)
| {post.featured && ( | ||
| <span className="featured-label bg-yellow-100 text-yellow-800 font-bold px-2 py-1 rounded-md absolute top-2 left-2 z-10"> | ||
| Featured | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix string quote style to maintain consistency.
The code uses double quotes for the className which violates the project's ESLint rules. Also, consider extracting these classes to a variable or constant for better readability if this style is used elsewhere.
- <span className="featured-label bg-yellow-100 text-yellow-800 font-bold px-2 py-1 rounded-md absolute top-2 left-2 z-10">
+ <span className='featured-label bg-yellow-100 text-yellow-800 font-bold px-2 py-1 rounded-md absolute top-2 left-2 z-10'>📝 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.
| {post.featured && ( | |
| <span className="featured-label bg-yellow-100 text-yellow-800 font-bold px-2 py-1 rounded-md absolute top-2 left-2 z-10"> | |
| Featured | |
| </span> | |
| )} | |
| {post.featured && ( | |
| <span className='featured-label bg-yellow-100 text-yellow-800 font-bold px-2 py-1 rounded-md absolute top-2 left-2 z-10'> | |
| Featured | |
| </span> | |
| )} |
🧰 Tools
🪛 ESLint
[error] 68-68: Replace "featured-label·bg-yellow-100·text-yellow-800·font-bold·px-2·py-1·rounded-md·absolute·top-2·left-2·z-10" with 'featured-label·bg-yellow-100·text-yellow-800·font-bold·px-2·py-1·rounded-md·absolute·top-2·left-2·z-10'
(prettier/prettier)
[error] 68-68: Unexpected usage of doublequote.
(jsx-quotes)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
markdown/about/index.md (3)
75-75: Vary Sentence Beginnings in the "Multicultural and inclusive" Section
The updated text repeats "It's" at the start of successive sentences. To improve readability and adhere to stylistic best practices, consider rephrasing one or more sentences to vary their openings. For example, you might merge or reword sentences to reduce the repetition.🧰 Tools
🪛 LanguageTool
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...always in constant mutation and change. It also actively demands compliance with h...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
95-95: Enhance Clarity in the "Blog" Section Description
The description for the Blog section now has correct apostrophes; however, starting both sentences with "It's" creates a repetitive rhythm. Consider merging the sentences or varying the structure to improve the flow. For example:- [**Blog**](https://www.asyncapi.com/blog): It's used to disseminate information related to the project. It's also open to collaborations. + [**Blog**](https://www.asyncapi.com/blog): Used to disseminate project-related information and open to collaborations.
96-96: Review Sentence Structure in the "LinkedIn" Section
Similarly, the LinkedIn section repeats the phrase "It's used" for both sentences. Consider combining or rewording the statements to create a more fluid description. For example:- [**LinkedIn**](https://es.linkedin.com/company/asyncapi): It's used to publish news related to the project or its environment. It's also used as a source of information. + [**LinkedIn**](https://es.linkedin.com/company/asyncapi): Publishes news related to the project and serves as a key source of information.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
markdown/about/index.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
markdown/about/index.md
[style] ~75-~75: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...always in constant mutation and change. It also actively demands compliance with h...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
⏰ Context from checks skipped due to timeout of 180000ms (3)
- GitHub Check: Redirect rules - asyncapi-website
- GitHub Check: Header rules - asyncapi-website
- GitHub Check: Pages changed - asyncapi-website
🔇 Additional comments (3)
markdown/about/index.md (3)
67-67: Consistent Company Description for SAP
The updated SAP entry now uses a consistent punctuation style with straight apostrophes. The description is clear and maintains uniformity with the rest of the companies section.
73-73: Typographical Fix in the "Free" Value
The "Free" value text now correctly uses straight apostrophes and maintains consistency. The sentence structure is clear.
97-97: Clear and Concise YouTube Channel Description
The updated description for the YouTube Channel is succinct and clear. No further changes are needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tests/build-pages.test.js (2)
52-55: Good addition of cleanup before test.This change properly ensures the test directory is clean before running the test, which prevents potential failures from previous test runs.
Consider using
fs.rmSync()with the recursive option for more robust directory cleanup:// Clean up directory if it exists before test if (fs.existsSync(NEW_TEST_DIR)) { - fs.rmdirSync(NEW_TEST_DIR); + fs.rmSync(NEW_TEST_DIR, { recursive: true, force: true }); }This would handle cases where the directory might contain files from a failed previous test run.
61-63: Good test cleanup practice.Adding cleanup after the test is completed ensures that test artifacts don't interfere with future test runs.
Similar to the pre-test cleanup, consider using
fs.rmSync()with the recursive option for more robust directory removal:// Clean up after test -fs.rmdirSync(NEW_TEST_DIR); +fs.rmSync(NEW_TEST_DIR, { recursive: true, force: true });scripts/dashboard/build-dashboard.js (1)
155-164: Improved month calculation logic.The new implementation of
monthsSinceprovides a more accurate calculation by properly accounting for year and month differences, along with a correction for the day of the month.Consider adding unit tests to verify the function works correctly with various date scenarios. For example:
// Test cases for monthsSince function test('monthsSince calculates correctly when current day is greater than target day', () => { const then = new Date(2022, 0, 15); // January 15, 2022 const now = new Date(2022, 3, 20); // April 20, 2022 // Mock the current date const originalDate = global.Date; global.Date = class extends Date { constructor() { return now; } }; expect(monthsSince(then)).toBe(3); // Restore original Date global.Date = originalDate; }); test('monthsSince calculates correctly when current day is less than target day', () => { const then = new Date(2022, 0, 25); // January 25, 2022 const now = new Date(2022, 3, 20); // April 20, 2022 // Mock the current date const originalDate = global.Date; global.Date = class extends Date { constructor() { return now; } }; expect(monthsSince(then)).toBe(2); // Should be 2 months due to day adjustment // Restore original Date global.Date = originalDate; });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/dashboard/build-dashboard.js(1 hunks)tests/build-pages.test.js(1 hunks)
🔇 Additional comments (1)
scripts/dashboard/build-dashboard.js (1)
97-97:Details
❓ Verification inconclusive
Verify impact of monthsSince changes on hot discussions scoring.
The changes to the
monthsSincefunction will potentially affect the scoring calculation for hot discussions, which uses this function in an exponentiated denominator. The improved accuracy is good, but it may change which discussions appear in the dashboard.
🏁 Script executed:
#!/bin/bash # Description: Check for usage of monthsSince in the codebase echo "Checking for monthsSince usage across the codebase:" rg -n "monthsSince" --glob="!scripts/dashboard/build-dashboard.js" echo "\nChecking if there are any tests for the monthsSince function:" rg -n "test.*monthsSince|describe.*monthsSince" --glob="*.test.js"Length of output: 525
Hot Discussion Scoring – Validate Impact of monthsSince Changes
The updated logic in
monthsSincenow improves accuracy, but its use in the hot discussions scoring (as seen inscripts/dashboard/build-dashboard.jsat line 97) might alter discussion rankings. Although there are tests (e.g., intests/dashboard/build-dashboard.test.jsaround lines 7 and 144) that referencemonthsSince, please verify that the updated calculation still meets the intended scoring behavior and product requirements. In particular:
- Double-check that the adjusted denominator exponentiation still produces expected scores for hot discussions.
- Ensure any changes in scoring due to enhanced accuracy are consistent with dashboard expectations.
- Confirm that regression tests (especially those in
tests/dashboard/build-dashboard.test.js) adequately cover these impacts.🧰 Tools
🪛 ESLint
[error] 97-97: 'monthsSince' was used before it was defined.
(no-use-before-define)
|
This issue will be considered under Bounty program hence closing this issue. |
3 participants
Description
added featured label (attached image for reference)
Integrated workflow with slack channel. have made slack-notifications.yml file inside .github/workflows (webhook URL might need to be changed)
Related issue(s)
solves #3778
Summary by CodeRabbit