-
-
Notifications
You must be signed in to change notification settings - Fork 265
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
Fix getVersionDescription() to prioritize version tags over non-version tags #673
Conversation
…on tags This fix modifies the getVersionDescription() method to ensure it only considers valid version tags when describing the current version. It retrieves all tags merged into the current branch, filters them based on a version-compatible regex, and uses the most recent valid version tag for description. If no valid tags are found, it falls back to the default description behavior. This resolves the issue of incorrect tags being used when multiple tags are present.
Warning Rate limit exceeded@OctopBP has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 26 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. 📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes involve a major update to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Versioning
participant Git
User->>Versioning: Request version description
Versioning->>Git: Fetch merged version tags
alt Valid tags found
Git-->>Versioning: Return valid tags
Versioning->>User: Return most recent tag description
else No valid tags found
Versioning->>Versioning: Log warning
Versioning->>Git: Fallback to git describe
Git-->>Versioning: Return version description
Versioning->>User: Return version description
end
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 (
|
Rewrote getting the description for the last valid tag using `rev-list` and `rev-parse`
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)
src/model/versioning.ts (3)
210-212
: Consider removing--merged HEAD
for broader tag coverage.The
--merged HEAD
option in thegit tag
command might exclude tags from other branches, potentially limiting the version description. Consider removing this option to include all tags:-const versionTags = (await this.git(['tag', '--list', '--merged', 'HEAD', '--sort=-creatordate'])) +const versionTags = (await this.git(['tag', '--list', '--sort=-creatordate']))This change would ensure that all tags are considered, providing a more comprehensive version description.
214-217
: Align fallback options with custom implementation.The fallback mechanism uses different options (
--long --tags --always
) compared to the custom implementation. Consider aligning these options or explaining the reason for the difference:-return this.git(['describe', '--long', '--tags', '--always', 'HEAD']); +return this.git(['describe', '--long', '--always', 'HEAD']);This change would make the fallback behavior more consistent with the custom implementation, unless there's a specific reason for including
--tags
in the fallback.
210-222
: Approve changes with minor suggestions.The new implementation of
getVersionDescription
successfully prioritizes version tags over non-version tags, addressing the main objective of the PR. It provides a more controlled and customized approach to generating version descriptions.However, consider the following suggestions for improvement:
- Add error handling for the
git
commands to gracefully handle potential failures.- Consider adding a comment explaining the format of the returned version string for better maintainability.
Overall, the changes look good and achieve the desired functionality.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/model/versioning.ts (1 hunks)
🔇 Additional comments (1)
src/model/versioning.ts (1)
210-222
: Addressing past review comments.@webbertakken The current implementation addresses your previous concerns:
- The
--tags
option is included in the fallback mechanism (line 216).- The custom implementation uses cross-platform compatible git commands.
The new approach should work in all scenarios on all platforms (Linux, macOS, Windows) with both detached head and normal situations. However, extensive testing across different environments is recommended to ensure full compatibility.
I also accidentally committed |
It is fine to commit the Before merging PRs, we usually make sure the I reviewed your changes, and they seem fine. I don't have too much context, but that looks good to me. Thanks for contributing, much appreciated! |
Released in v4.3.0. |
Fetching Version Tags:
The command
git tag --list --merged HEAD --sort=-creatordate
retrieves all tags that have been merged into the current branch, sorted by creation date (newest first).We then filter these tags to keep only those that match the
grepCompatibleInputVersionRegex
, which defines valid version tags (like1.2
).Handling No Version Tags:
If no version tags are found, we fall back to using git describe with
--long
and--always
to provide a generic description.Using Latest Version Tag:
After filtering the tags, the latest valid tag is used in the git describe command, which ensures we describe the version based on the last suitable tag.
Summary by CodeRabbit
New Features
Bug Fixes