chore: release automations #33
Conversation
WalkthroughThe changes in this pull request involve the removal of several GitHub Actions workflows related to translation, release management, and pull request validation, as well as the introduction of new workflows for building and publishing Python packages. The new workflows streamline the process of versioning and publishing to PyPI, incorporating scripts to manage version increments for major, minor, and build versions. Additionally, scripts for removing alpha designations and for bumping version numbers have been added to enhance version management. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 24
🧹 Outside diff range and nitpick comments (13)
scripts/remove_alpha.py (3)
1-3: LGTM! Consider using a more robust path construction method.The import statements and file path construction look good. However, to make the script more robust and portable, consider using
Pathfrom thepathlibmodule for path manipulation.Here's a suggested improvement:
-import fileinput -from os.path import join, dirname -version_file = join(dirname(dirname(__file__)), "version.py") +import fileinput +from pathlib import Path +version_file = Path(__file__).parent.parent / "version.py"This change makes the path construction more readable and less prone to errors on different operating systems.
4-5: LGTM! Consider adding error handling for file operations.The setup for file iteration and the
alpha_var_namevariable declaration are correct. However, it's important to handle potential file operation errors.Consider wrapping the file operations in a try-except block to handle potential IOErrors:
try: for line in fileinput.input(version_file, inplace=True): # ... (rest of the code) except IOError as e: print(f"Error processing file: {e}") sys.exit(1)This change will make the script more robust by gracefully handling file-related errors.
6-9: LGTM! Consider a minor improvement for clarity.The logic for modifying the file content is correct. It effectively sets
VERSION_ALPHAto 0 and preserves other lines.For improved clarity, consider using an explicit comparison instead of
startswith():- if line.startswith(alpha_var_name): + if line.strip().startswith(f"{alpha_var_name} ="): print(f"{alpha_var_name} = 0") else: print(line.rstrip('\n'))This change makes it clearer that we're looking for the variable assignment line, not just any line starting with the variable name.
scripts/bump_alpha.py (2)
1-5: LGTM! Consider parameterizing the version variable name.The imports and file path setup look good. The relative path construction is a good practice for portability.
Consider making the
version_var_namea command-line argument or a configuration value for increased flexibility if the variable name needs to change in the future.
1-15: Overall assessment: Functional but needs robustness improvements.The script successfully increments the alpha version, but there are several areas for improvement:
- Error handling: Add comprehensive error checks throughout the script.
- File safety: Implement file backups and safe write operations.
- Logging: Add logging statements for better traceability and debugging.
- Parameterization: Consider making the script more flexible by accepting command-line arguments.
To enhance the script's robustness and maintainability, consider refactoring it into a main function with smaller, focused helper functions. This would improve readability and make it easier to add unit tests.
Here's a high-level structure suggestion:
import argparse import logging def parse_arguments(): # Parse command-line arguments def read_version(file_path, var_name): # Read and return the current version def update_version(file_path, var_name, new_version): # Update the version in the file def main(): args = parse_arguments() setup_logging(args.verbose) try: current_version = read_version(args.file, args.var_name) new_version = current_version + 1 update_version(args.file, args.var_name, new_version) logging.info(f"Version bumped to {new_version}") except Exception as e: logging.error(f"Error: {str(e)}") return 1 return 0 if __name__ == "__main__": exit(main())This structure allows for better error handling, logging, and future extensibility.
scripts/bump_major.py (1)
13-23: LGTM: File modification logic is sound, but consider adding error handling.The file modification logic using
fileinputis well-implemented. It correctly updates the major version and resets other version components while preserving the file structure.Consider adding error handling for potential IOErrors during file writing:
try: for line in fileinput.input(version_file, inplace=True): # ... (existing logic) except IOError as e: print(f"Error updating {version_file}: {e}") sys.exit(1)This addition will make the script more robust against potential file system issues.
.github/workflows/publish_alpha.yml (1)
Line range hint
1-19: Consider a more specific workflow nameThe current name "Publish Alpha Build ...aX" is descriptive but could be more specific. Consider updating it to "Publish Python Package Alpha Build" to clearly indicate the nature of the package being published.
🧰 Tools
🪛 actionlint
36-36: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
51-51: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
.github/workflows/publish_build.yml (1)
2-2: Fix the workflow nameThe workflow name ends with "..X", which appears to be a typo or placeholder. Consider removing it for a cleaner, more professional name.
Apply this change:
-name: Publish Build Release ..X +name: Publish Build Release.github/workflows/publish_minor.yml (3)
6-19: LGTM: Job setup and initial steps are well-configured.The job configuration and initial steps are appropriate for the task. However, consider using a more recent Python version for potential performance improvements and longer support.
Consider updating the Python version to 3.11 or 3.12 for improved performance and longer support:
- python-version: 3.10 + python-version: 3.12
76-84: LGTM: Matrix notification step is well-implemented.Sending a notification to a Matrix channel about the new release is a good practice for team communication. The implementation is secure, using a secret for the Matrix token.
Consider enhancing the notification message with a link to the GitHub release for easy access to the changelog:
message: | - New skill-ovos-fallback-unknown release! ${{ steps.version.outputs.version }} + New skill-ovos-fallback-unknown release! ${{ steps.version.outputs.version }} + Release details: https://github.com/${{ github.repository }}/releases/tag/V${{ steps.version.outputs.version }}
1-84: Overall, the workflow is well-designed for automating minor releases.This GitHub Actions workflow effectively automates the process of publishing minor releases. It covers all necessary steps including:
- Version management
- Changelog generation
- GitHub release creation
- Distribution package building
- PyPI publishing
- Team notification
The workflow follows good practices such as using secrets for sensitive information, leveraging official actions where possible, and maintaining both
devandmasterbranches.While there are a few minor issues with deprecated commands and unused variables (as noted in previous comments), these can be easily addressed. Once these small updates are made, this workflow will provide a robust and efficient process for managing minor releases.
To further improve this workflow, consider the following suggestions:
- Add a step to run tests before building and publishing, ensuring only validated code is released.
- Implement a mechanism to automatically update the project's documentation with the new version number.
- Consider adding a step to create and push git tags for each release, if not already handled by the
actions/create-releaseaction.🧰 Tools
🪛 actionlint
21-21: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
41-41: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
61-61: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
64-64: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
.github/workflows/publish_major.yml (2)
7-19: Consider updating to a more recent Python version.The workflow setup looks good overall. However, you might want to consider using a more recent Python version, such as 3.11 or 3.12, to take advantage of the latest features and performance improvements.
- python-version: 3.10 + python-version: '3.12'This change would ensure you're using the latest stable Python version for your build process.
76-84: LGTM: Matrix notification is a good practice. Consider adding more details to the message.The Matrix notification step is a great way to keep the team informed about new releases. To make the notification even more useful, consider adding more details to the message, such as:
- A link to the GitHub release page.
- A brief summary of key changes or features in this release.
Here's an example of how you could enhance the message:
message: | New skill-ovos-fallback-unknown release! ${{ steps.version.outputs.version }} Release page: https://github.com/${{ github.repository }}/releases/tag/V${{ steps.version.outputs.version }} Key changes: - [Add a brief summary of major changes or features] For full details, please check the release page.This additional information would provide more context to the team about the new release.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
- .github/workflows/auto_translate.yml (0 hunks)
- .github/workflows/publish_alpha.yml (2 hunks)
- .github/workflows/publish_build.yml (1 hunks)
- .github/workflows/publish_major.yml (1 hunks)
- .github/workflows/publish_minor.yml (1 hunks)
- .github/workflows/publish_release.yml (0 hunks)
- .github/workflows/pull-request-lint.yml (0 hunks)
- .github/workflows/test_resources.yml (0 hunks)
- scripts/bump_alpha.py (1 hunks)
- scripts/bump_build.py (1 hunks)
- scripts/bump_major.py (1 hunks)
- scripts/bump_minor.py (1 hunks)
- scripts/remove_alpha.py (1 hunks)
💤 Files with no reviewable changes (4)
- .github/workflows/auto_translate.yml
- .github/workflows/publish_release.yml
- .github/workflows/pull-request-lint.yml
- .github/workflows/test_resources.yml
🧰 Additional context used
🪛 actionlint
.github/workflows/publish_alpha.yml
36-36: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
51-51: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
.github/workflows/publish_build.yml
21-21: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
41-41: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
61-61: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
64-64: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
.github/workflows/publish_major.yml
21-21: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
41-41: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
61-61: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
64-64: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
.github/workflows/publish_minor.yml
21-21: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
41-41: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
61-61: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
64-64: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
🔇 Additional comments (10)
scripts/bump_build.py (1)
1-5: LGTM: Imports and variable definitions are appropriate.The imports and variable definitions are well-chosen for the script's purpose. The use of
os.pathfunctions ensures cross-platform compatibility when constructing file paths.scripts/bump_minor.py (1)
1-6: LGTM: Imports and variable definitions are appropriate.The imports and variable definitions are well-structured and provide a clear setup for the script's functionality. The use of
os.pathfunctions ensures cross-platform compatibility.scripts/bump_major.py (1)
1-7: LGTM: Imports and variable declarations are appropriate.The imports and variable declarations are well-structured and provide a clear foundation for the script's functionality. The use of
os.pathfunctions ensures cross-platform compatibility..github/workflows/publish_alpha.yml (1)
53-66: Release creation step looks goodThe release creation step is well-configured. It correctly uses the incremented version number, includes the generated changelog, and is appropriately marked as a prerelease for an alpha build.
.github/workflows/publish_build.yml (1)
76-84: LGTM: Matrix notification stepThe step for sending a notification to the Matrix channel after a successful release is well-implemented. It provides good visibility for the team about new releases.
.github/workflows/publish_minor.yml (3)
1-4: LGTM: Workflow name and trigger are appropriate.The workflow name clearly describes its purpose, and the manual trigger via
workflow_dispatchis suitable for a release workflow, allowing control over when the release process is initiated.
72-75: LGTM: PyPI publishing step is well-configured.The use of the official
pypa/gh-action-pypi-publishaction for publishing to PyPI is a good practice. The PyPI token is correctly referenced as a secret, ensuring secure authentication.
43-71:⚠️ Potential issueRelease creation steps are well-structured but need minor updates.
The release creation process is comprehensive, covering GitHub release creation, package building, and version incrementing. However, there are a couple of issues to address:
- The
set-outputcommand is deprecated.- The
VERvariable in the "Increment Version" step is unused.Please apply the following changes:
- Update the deprecated
set-outputcommand:- run: echo "::set-output name=version::$(python setup.py --version)" + run: echo "version=$(python setup.py --version)" >> $GITHUB_OUTPUT
- Remove the unused
VERvariable:- VER=$(python setup.py --version) python scripts/bump_minor.pyTo ensure the
bump_minor.pyscript is functioning correctly, let's examine its contents:#!/bin/bash cat scripts/bump_minor.py🧰 Tools
🪛 actionlint
61-61: workflow command "set-output" was deprecated. use
echo "{name}={value}" >> $GITHUB_OUTPUTinstead: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions(deprecated-commands)
64-64: shellcheck reported issue in this script: SC2034:warning:1:1: VER appears unused. Verify use (or export if used externally)
(shellcheck)
.github/workflows/publish_major.yml (2)
1-4: LGTM: Workflow name and trigger are appropriate.The workflow name clearly indicates its purpose for publishing major releases, and the manual trigger via
workflow_dispatchprovides necessary control over when to initiate the release process.
72-75: LGTM: PyPI publishing step is secure and follows best practices.The PyPI publishing step uses the official PyPA action and securely handles the PYPI_TOKEN. This is the recommended way to publish packages to PyPI from GitHub Actions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
requirements-dev.txt (1)
1-2: Consider specifying versions and adding a file descriptionTo ensure consistency across different development environments and prevent potential compatibility issues, it's recommended to specify the versions of the dependencies. Additionally, adding a comment at the top of the file explaining its purpose can be helpful for new contributors.
Here's a suggested improvement:
+# Development dependencies for testing and code coverage -pytest -pytest-cov +pytest==6.2.5 +pytest-cov==2.12.1Note: The versions provided are examples. Please use the most appropriate versions for your project.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- requirements-dev.txt (1 hunks)
- setup.py (1 hunks)
- test/test_skill.py (0 hunks)
💤 Files with no reviewable changes (1)
- test/test_skill.py
🧰 Additional context used
🔇 Additional comments (2)
requirements-dev.txt (1)
1-2: Excellent addition of testing dependencies!The inclusion of
pytestandpytest-covis a great step towards improving the project's testing capabilities. These tools will enable developers to write and run tests more effectively, as well as measure code coverage, which are crucial for maintaining code quality.setup.py (1)
81-81: LGTM! Great addition for managing test dependencies.This change adds an
extras_requireparameter to thesetup()function, which is an excellent practice for Python packaging. It allows for the separation of development/testing dependencies from the main package requirements. Users can now install these additional dependencies usingpip install skill-laugh[test].Benefits of this change:
- Clearer separation of concerns between production and development dependencies.
- Easier setup for contributors who want to run tests.
- Reduced package size for end-users who don't need testing tools.
The implementation is consistent with the existing code, using the
get_requirements()function to read from "requirements-dev.txt".
Closes #20
Summary by CodeRabbit
New Features
Bug Fixes
Chores
pytestandpytest-covto the development requirements for improved testing capabilities.