-
Notifications
You must be signed in to change notification settings - Fork 24
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
ci: post to mastodon #201
ci: post to mastodon #201
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
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: 0
🧹 Nitpick comments (2)
.github/workflows/post_to_mastodon.sh (2)
9-14
: Consider a more flexible release PR pattern.While the current pattern works for the exact format
chore(main): release
, consider making it more flexible to handle variations like:
- Different scopes (e.g.,
chore(release)
)- Different prefixes (e.g.,
release:
,feat:
)- Case variations
-if [[ ! "${PR_TITLE}" =~ ^chore\(main\):\ release ]]; then +if [[ ! "${PR_TITLE}" =~ [Rr]elease[[:space:]] ]]; then
15-17
: Consider using regex capture for more robust version extraction.While the current approach works for simple cases, it might include unwanted text if there's anything after the version number. Consider using regex capture to extract exactly the version number.
-# Extract version (everything after "release ") -version="${PR_TITLE#*release }" +# Extract version using regex capture +if [[ "${PR_TITLE}" =~ [Rr]elease[[:space:]]+([0-9]+\.[0-9]+\.[0-9]+) ]]; then + version="${BASH_REMATCH[1]}" +else + echo "Error: Could not extract version number from PR title" + exit 1 +fiThis approach:
- Extracts exactly the version number
- Validates the format during extraction
- Provides better error handling
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/post_to_mastodon.sh
(1 hunks)
🧰 Additional context used
📓 Learnings (1)
.github/workflows/post_to_mastodon.sh (1)
Learnt from: cmeesters
PR: snakemake/snakemake-executor-plugin-slurm#193
File: .github/workflows/post_to_mastodon.yml:0-0
Timestamp: 2025-01-20T09:24:03.959Z
Learning: In the snakemake-executor-plugin-slurm repository's post_to_mastodon.sh script, the version number is extracted from PR titles using the parameter expansion `${PR_TITLE##* }`, which removes everything up to the last space in the string.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: testing
🔇 Additional comments (1)
.github/workflows/post_to_mastodon.sh (1)
4-8
: LGTM! Good environment variable validation.The early validation of the required
PR_TITLE
environment variable with a clear error message is a good practice.
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: 0
🧹 Nitpick comments (2)
.github/workflows/post_to_mastodon.sh (2)
9-13
: Consider making the PR title pattern more flexible.The current regex pattern might be too restrictive:
- It requires "main" as the scope, which might not work for releases from other branches.
- It's case-sensitive, which could cause issues with different casing.
Consider this more flexible pattern:
-if [[ ! "${PR_TITLE}" =~ ^chore\(main\):\ release ]]; then +if [[ ! "${PR_TITLE}" =~ ^[Cc]hore\([^)]+\):\ [Rr]elease ]]; then
15-21
: Fix potential edge case in version extraction.The current regex pattern requires whitespace after the version number, which might fail if the version is at the end of the PR title.
Consider this more robust pattern:
-if [[ "${PR_TITLE}" =~ [Rr]elease[[:space:]]+([0-9]+\.[0-9]+\.[0-9]+) ]]; then +if [[ "${PR_TITLE}" =~ [Rr]elease[[:space:]]+([0-9]+\.[0-9]+\.[0-9]+)[[:space:]]*$ ]]; thenThis pattern ensures it matches versions at the end of the title by:
- Making the trailing whitespace optional with
[[:space:]]*
- Anchoring to the end of the string with
$
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/post_to_mastodon.sh
(1 hunks)
🧰 Additional context used
📓 Learnings (1)
.github/workflows/post_to_mastodon.sh (1)
Learnt from: cmeesters
PR: snakemake/snakemake-executor-plugin-slurm#193
File: .github/workflows/post_to_mastodon.yml:0-0
Timestamp: 2025-01-20T09:24:03.959Z
Learning: In the snakemake-executor-plugin-slurm repository's post_to_mastodon.sh script, the version number is extracted from PR titles using the parameter expansion `${PR_TITLE##* }`, which removes everything up to the last space in the string.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: testing
🔇 Additional comments (2)
.github/workflows/post_to_mastodon.sh (2)
4-8
: LGTM! Good error handling.The validation of the
PR_TITLE
environment variable is well-implemented with a clear error message and appropriate exit status.
9-21
: Verify PR title patterns against repository history.Let's ensure the new PR title validation aligns with the repository's conventions.
✅ Verification successful
PR title pattern validation is appropriate for new release workflow
The repository is new (created September 2023) with no release history yet. The implemented pattern
chore(main): release X.Y.Z
follows conventional commits format and is a standard approach for release commits.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check recent PR titles to validate our patterns # Get recent PR titles echo "Recent PR titles:" gh pr list --limit 10 --json title --jq '.[].title' # Count release PRs with current pattern echo -e "\nRelease PRs (current pattern):" gh pr list --limit 50 --json title --jq '.[] | select(.title | test("^chore\\(main\\):\\s+release\\s+\\d+\\.\\d+\\.\\d+\\s*$")) | .title' # Count release PRs with suggested pattern echo -e "\nRelease PRs (suggested pattern):" gh pr list --limit 50 --json title --jq '.[] | select(.title | test("^[Cc]hore\\([^)]+\\):\\s+[Rr]elease\\s+\\d+\\.\\d+\\.\\d+\\s*$")) | .title'Length of output: 613
Script:
#!/bin/bash # Get repository info and release history echo "Repository creation date:" gh repo view --json createdAt --jq '.createdAt' echo -e "\nGit tags (latest 10):" git tag -l --sort=-creatordate | head -n 10 echo -e "\nPRs containing 'release' (case insensitive):" gh pr list --limit 100 --search "release in:title" --json title --jq '.[].title'Length of output: 428
next attempt: filtering out the pr title string, hopefully corrected
Summary by CodeRabbit