-
Notifications
You must be signed in to change notification settings - Fork 637
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
refactor(semver): make outside()
more readable
#4095
Conversation
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.
Are you able to add the test case mentioned in #3948 (comment) ?
Done. |
Actually, would it make sense to additionally deprecate |
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.
LGTM, as this removes the dependency on outside()
, which might be worth deprecating.
Please see the comment on the new test that I've added. We should relocate, then tweak this test if the reported behaviour is agreed upon to be a bug (which both Tim and I already do).
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.
LGTM
outside()
more readable by replacing the algorithm with already existing functions.As pointed out in #3948 (comment), this seems now to be a weird function to include in the semver mod.