-
Notifications
You must be signed in to change notification settings - Fork 6
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: use dynamic versioning #24
Conversation
Warning Rate limit exceeded@aaronsteers has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes in the 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: 0
🧹 Outside diff range and nitpick comments (1)
pyproject.toml (1)
27-28
: Consider additional version format configuration.While the basic configuration is correct, consider adding these optional configurations for more control:
style
to specify version format (pep440, semver, etc.)pattern
to customize version regex patternformat
to customize version format stringExample configuration:
[tool.poetry-dynamic-versioning] enable = true +style = "pep440" +format = "{base}" +pattern = "^(?P<base>\\d+\\.\\d+\\.\\d+)$"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
pyproject.toml
(2 hunks)
🔇 Additional comments (2)
pyproject.toml (2)
2-3
: LGTM! Version constraints follow best practices.
The build system changes are well-structured:
- Version constraint
>=1.0.0,<2.0.0
follows best practices by allowing patches while preventing breaking changes - Build backend change is required for dynamic versioning to work
Let's verify we're using the latest stable version:
24-25
: LGTM! Clear documentation of version management strategy.
The version change and documentation are well-implemented:
- Setting version to "0.0.0" is correct for dynamic versioning
- Comment clearly explains the versioning strategy and links to documentation
Let's verify there are no hardcoded version references:
✅ Verification successful
Version reference in CHANGELOG.md is historical and should be kept
The version "6.5.2" appears only in the CHANGELOG.md file as part of the version history, which is the correct location for it. This is not a hardcoded reference that needs updating, as changelogs should maintain the historical record of previous versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any hardcoded references to version 6.5.2 that might need updating
rg -l "6\.5\.2" --type-not toml
Length of output: 46
Script:
#!/bin/bash
# Check the context of version 6.5.2 in CHANGELOG.md
rg -A 5 "6\.5\.2" CHANGELOG.md
Length of output: 133
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
🧹 Outside diff range and nitpick comments (3)
.github/workflows/pypi_publish.yml (3)
1-1
: Consider a more descriptive workflow nameWhile "PyPI" is concise, consider a name that better describes the workflow's purpose, such as "PyPI Build & Publish" or "Package Release".
26-27
: Add URL validation and documentationWhile using an environment variable for the PyPI URL provides flexibility, consider:
- Adding URL validation to prevent misconfiguration
- Documenting the expected URL format in the repository
Add this step before the publish job:
- name: Validate PyPI URL run: | if [[ ! "$PYPI_PUBLISH_URL" =~ ^https://(test\.)?pypi\.org/p/airbyte$ ]]; then echo "Invalid PYPI_PUBLISH_URL: $PYPI_PUBLISH_URL" exit 1 fi
Line range hint
1-46
: Consider implementing release environment strategyThe current setup allows toggling between production and test PyPI through an environment variable. Consider implementing a more structured approach using GitHub Environments:
- Create separate "PyPI-Test" and "PyPI-Production" environments
- Define environment-specific variables and protection rules
- Use environment conditions in the workflow
This would provide better separation of concerns and enhanced security controls.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/pypi_publish.yml
(4 hunks)
🔇 Additional comments (2)
.github/workflows/pypi_publish.yml (2)
45-46
: Verify environment variable configuration
The repository-url configuration looks correct, but ensure the PYPI_PUBLISH_URL
environment variable is properly configured in the repository settings.
Let's check for any existing environment variable documentation:
#!/bin/bash
# Description: Look for environment variable documentation
# Expected: Documentation mentioning PYPI_PUBLISH_URL configuration
rg -l "PYPI_PUBLISH_URL" --type md
fd -e md -x cat {} \; | rg -A 5 -B 5 "PYPI_PUBLISH_URL" || true
Line range hint 10-16
: LGTM! Build configuration is well structured
The use of hynek/build-and-inspect-python-package@v2
with full history fetch is appropriate for dynamic versioning.
Let's verify the action's compatibility with poetry-dynamic-versioning:
✅ Verification successful
LGTM! Build configuration is compatible with poetry-dynamic-versioning
The project correctly uses poetry-dynamic-versioning:
- Configured in
pyproject.toml
withpoetry-dynamic-versioning>=1.0.0,<2.0.0
in build requirements - Uses
poetry_dynamic_versioning.backend
as the build backend - Has dynamic versioning enabled with
[tool.poetry-dynamic-versioning]
- The
fetch-depth: 0
in the workflow ensures full git history for version calculation
The hynek/build-and-inspect-python-package@v2
action will work correctly with this setup as it's a standard build process that respects the configured build backend.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the build action supports poetry-dynamic-versioning
# Expected: Documentation or issues mentioning poetry-dynamic-versioning support
gh api repos/hynek/build-and-inspect-python-package/issues \
--jq '.[] | select(.body + .title | contains("poetry-dynamic-versioning"))' || true
Length of output: 315
Script:
#!/bin/bash
# Let's verify if poetry-dynamic-versioning is configured in the project
fd -e toml -x cat {} \; | grep -A 5 -B 5 "poetry-dynamic-versioning"
# Check the build system requirements
fd pyproject.toml -x cat {} \; | grep -A 10 "\[build-system\]"
# Look for any dynamic version configuration
fd -e toml -x cat {} \; | grep -A 5 "dynamic"
# Check workflow files for any poetry setup steps
fd "\.ya?ml$" .github/workflows -x cat {} \; | grep -A 5 -B 5 "poetry"
Length of output: 13454
Summary by CodeRabbit
New Features
Bug Fixes
Chores