-
Notifications
You must be signed in to change notification settings - Fork 151
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
chore(automation): add workflow to automatically update CONTRIBUTORS.md #596
Conversation
WalkthroughA new GitHub Actions workflow, Changes
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
CodeRabbit Configuration File (
|
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: 5
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- .github/workflows/update_contributors.yml (1 hunks)
- scripts/update_contributors.py (1 hunks)
Additional context used
actionlint
.github/workflows/update_contributors.yml
25-25: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
30-30: shellcheck reported issue in this script: SC2086:info:1:59: Double quote to prevent globbing and word splitting
(shellcheck)
Additional comments not posted (3)
scripts/update_contributors.py (1)
1-3
: Review import statements for necessity and security.The script imports
sys
andos
which are necessary for its functionality. However, always ensure that the use of such modules does not introduce any security vulnerabilities, especially when dealing with file paths and external inputs..github/workflows/update_contributors.yml (2)
3-10
: Validate trigger and condition logic.The workflow is correctly triggered on closed pull requests and includes a condition to only run if the pull request was merged. This is a good use of GitHub Actions features to ensure the workflow runs only when necessary.
15-19
: Review Node.js setup necessity.The workflow sets up Node.js but does not appear to use it in any subsequent steps, as the script being run is a Python script. Verify whether Node.js is actually needed, as this could reduce the runtime and complexity of the workflow.
scripts/update_contributors.py
Outdated
contributor = sys.argv[1] | ||
pr_count = int(sys.argv[2]) | ||
|
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.
Validate command-line arguments for robustness.
The script directly uses sys.argv
for input without any form of validation. Consider adding checks to ensure that the script is not run with insufficient or malformed arguments, which could lead to runtime errors or unexpected behavior.
Suggest adding argument validation:
if len(sys.argv) < 3:
print("Usage: python update_contributors.py <contributor> <pr_count>")
sys.exit(1)
try:
pr_count = int(sys.argv[2])
except ValueError:
print("Error: pr_count must be an integer.")
sys.exit(1)
scripts/update_contributors.py
Outdated
tiers = { | ||
"Valued Contributor": 10, | ||
"Frequent Contributor": 5, | ||
"First Contributor": 1 | ||
} |
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.
Review the tier system for scalability and maintainability.
The tier system is hardcoded, which might limit flexibility if the tier thresholds or names need to be updated frequently. Consider externalizing these values to a configuration file or environment variables for easier management without modifying the code.
scripts/update_contributors.py
Outdated
contributors_file_path = os.path.join(os.path.dirname(__file__), '..', 'CONTRIBUTORS.md') | ||
|
||
with open(contributors_file_path, "r") as file: | ||
lines = file.readlines() | ||
|
||
found = False | ||
new_lines = [] | ||
for line in lines: | ||
if contributor in line: | ||
found = True | ||
for tier, count in tiers.items(): | ||
if pr_count >= count: | ||
line = f"| @{contributor} | ![{tier} Badge](https://img.shields.io/badge/{tier.replace(' ', '%20')}-Achieved-blue) |\n" | ||
break | ||
new_lines.append(line) | ||
|
||
if not found: | ||
for tier, count in tiers.items(): | ||
if pr_count >= count: | ||
new_lines.append(f"| @{contributor} | ![{tier} Badge](https://img.shields.io/badge/{tier.replace(' ', '%20')}-Achieved-blue) |\n") | ||
break | ||
|
||
with open(contributors_file_path, "w") as file: | ||
file.writelines(new_lines) |
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.
Ensure proper handling of file operations and potential errors.
The script reads and writes to the CONTRIBUTORS.md file but does not handle potential I/O errors such as file not found or permission issues. It's crucial to add error handling around file operations to prevent the script from failing silently or crashing.
Add error handling for file operations:
try:
with open(contributors_file_path, "r") as file:
lines = file.readlines()
except IOError as e:
print(f"Error reading file: {e}")
sys.exit(1)
# Further operations...
try:
with open(contributors_file_path, "w") as file:
file.writelines(new_lines)
except IOError as e:
print(f"Error writing to file: {e}")
sys.exit(1)
run: | | ||
PR_COUNT=$(gh pr list --author ${{ github.actor }} --state merged --repo ${{ github.repository }} --json number --jq '. | length') | ||
echo "PR_COUNT=$PR_COUNT" >> $GITHUB_ENV | ||
|
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.
Address shell script issues flagged by static analysis.
Static analysis tools flagged potential issues with word splitting and globbing due to unquoted variables. This can lead to unexpected behavior if variables contain spaces or special characters.
Apply this fix to address the shell script issues:
run: |
PR_COUNT=$(gh pr list --author "${{ github.actor }}" --state merged --repo "${{ github.repository }}" --json number --jq '. | length')
echo "PR_COUNT=$PR_COUNT" >> $GITHUB_ENV
Tools
actionlint
25-25: shellcheck reported issue in this script: SC2086:info:2:30: Double quote to prevent globbing and word splitting
(shellcheck)
run: | | ||
python scripts/update_contributors.py ${{ github.actor }} $PR_COUNT | ||
|
||
- name: Commit and Push Changes to Dedicated Branch | ||
run: | | ||
git config --local user.email "github-actions[bot]@users.noreply.github.com" | ||
git config --local user.name "github-actions[bot]" | ||
git checkout -B contributors-updates | ||
git add CONTRIBUTORS.md | ||
git commit -m "Update CONTRIBUTORS.md for ${{ github.actor }}" | ||
git push --set-upstream origin contributors-updates |
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.
Ensure robustness in git operations.
The steps to commit and push changes are well-defined. However, consider adding error handling or conditions to ensure that changes are only pushed if there are actual updates to CONTRIBUTORS.md, to avoid empty commits.
Add a check before committing to ensure there are changes:
run: |
git config --local user.email "github-actions[bot]@users.noreply.github.com"
git config --local user.name "github-actions[bot]"
git checkout -B contributors-updates
git add CONTRIBUTORS.md
if git diff --staged --quiet; then
echo "No changes to CONTRIBUTORS.md, skipping commit."
else
git commit -m "Update CONTRIBUTORS.md for ${{ github.actor }}"
git push --set-upstream origin contributors-updates
Tools
actionlint
30-30: shellcheck reported issue in this script: SC2086:info:1:59: Double quote to prevent globbing and word splitting
(shellcheck)
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- scripts/update_contributors.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- scripts/update_contributors.py
This approach is a bit heavy handed and we solved this with a manual approach which was better suited. |
Purpose: This PR introduces a new GitHub Actions workflow and accompanying Python script to automate the process of updating the CONTRIBUTORS.md file. This ensures that contributor tiers and badges are updated automatically based on the number of merged PRs.
Changes:
Impact:
Next Steps:
Summary by CodeRabbit
New Features
Bug Fixes