-
Notifications
You must be signed in to change notification settings - Fork 43
The compareMultiSemver
function now uses the embedded compareSemver
#794
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
base: main
Are you sure you want to change the base?
Conversation
function instead of importing it from an external 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.
Orca Security Scan Summary
Status | Check | Issues by priority | |
---|---|---|---|
![]() |
Infrastructure as Code | ![]() ![]() ![]() ![]() |
View in Orca |
![]() |
SAST | ![]() ![]() ![]() ![]() |
View in Orca |
![]() |
Secrets | ![]() ![]() ![]() ![]() |
View in Orca |
![]() |
Vulnerabilities | ![]() ![]() ![]() ![]() |
View in Orca |
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.
✨ PR Review
The PR successfully embeds the compareSemver function to eliminate external dependency, but introduces test code in the production file and has potential issues with semver comparison logic.
2 issues detected:
🧹 Maintainability - Test code should not be part of production modules as it creates unnecessary coupling and potential runtime overhead.
Details: The file now contains test cases and console output statements that are executed as part of the main module. This makes the production code noisy and couples testing logic with business logic.
File:plugins/filters/compareMultiSemver/index.js
🐞 Bug - Using character codes for version comparison can lead to incorrect semantic version ordering.
Details: The convertPart function uses charCodeAt() to convert alphabetic characters to numeric values, which can create inconsistent ordering for prerelease versions. Character codes vary significantly (a=97, z=122) leading to unexpected comparison results.
File:plugins/filters/compareMultiSemver/index.js (55-56)
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using. We'd love your feedback! 🚀
const match = /[A-Za-zαß]/.exec(x); | ||
return Number(match ? x.replace(match[0], "." + x.charCodeAt(match.index)) : x); |
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.
🐞 Bug - Inconsistent Prerelease Handling: Implement proper semver prerelease comparison logic or use a standardized approach for handling alphabetic version suffixes instead of character codes.
const match = /[A-Za-zαß]/.exec(x); | |
return Number(match ? x.replace(match[0], "." + x.charCodeAt(match.index)) : x); | |
const match = /^(\d+)([A-Za-zαß]*)$/.exec(x); | |
if (!match) return Number(x); | |
const numeric = Number(match[1]); | |
const alpha = match[2]; | |
if (!alpha) return numeric; | |
// Convert alphabetic suffix to a small decimal increment for proper ordering | |
// Each character contributes progressively smaller values to maintain order | |
let alphaValue = 0; | |
for (let i = 0; i < alpha.length; i++) { | |
const charValue = alpha.charCodeAt(i) - 96; // normalize a=1, b=2, etc. | |
alphaValue += charValue / Math.pow(100, i + 1); // decimal fractions | |
} | |
return numeric + alphaValue; |
function instead of importing it from an external file.
✨ PR Description
Purpose: Embed compareSemver functionality directly into compareMultiSemver plugin to eliminate external dependency and improve maintainability.
Main changes:
Generated by LinearB AI and added by gitStream.
AI-generated content may contain inaccuracies. Please verify before using. We'd love your feedback! 🚀