-
Notifications
You must be signed in to change notification settings - Fork 83
feat: semver implementation #571
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
Conversation
* | ||
*/ | ||
function isNumber(content: string): boolean { | ||
return new RegExp(/^\d+$/).test(content); |
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.
remove RegExp
/^\d+$/.test(content)
* | ||
*/ | ||
function hasWhiteSpaces(version: string): boolean { | ||
return new RegExp(/\s/).test(version); |
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.
/\s/.test(version)
* @return {boolean} The array of version split into smaller parts i.e major, minor, patch etc | ||
* null if given version is in invalid format | ||
*/ | ||
function splitVersion(version: string): string[] | null { |
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.
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.
@mjc1283 I think, parsing condition would be bit difficult if we return SemVer interface like you said in the comment.
One of the reason, we are allowing all variations. Version can have the following forms.
major
major.minor
major.minor.patch
major.minor.patch.pre-release
major.minor.patch.pre-release+build
major.minor.patch+build
* null if invalid user or condition version is provided | ||
*/ | ||
export function compareVersion(conditionsVersion: string, userProvidedVersion: string): number | null { | ||
const isPreReleaseInconditionsVersion = isPreReleaseVersion(conditionsVersion); |
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.
I think we don't really need this variable, let me think. btw
x isPreReleaseInConditionsVersion
*/ | ||
function greaterThanEvaluator(condition: Condition, userAttributes: UserAttributes): boolean | null { | ||
function validateValues(condition: Condition, userAttributes: UserAttributes): boolean { |
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.
function validateValues(condition: Condition, userAttributes: UserAttributes): boolean { | |
function validateValuesForNumericCondition(condition: Condition, userAttributes: UserAttributes): boolean { |
@@ -0,0 +1,190 @@ | |||
/** |
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.
This file has several style/formatting issues. Please see other files and follow the same style for indentation, brackets on the same line as if
statements, always use brackets with if
statements. I will try update the linter to enforce these.
Summary
Test plan