Skip to content
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

Add ~ and ^ semver semantic to nimble "requires" #908

Merged
merged 9 commits into from
Jul 4, 2021

Conversation

marcomq
Copy link
Contributor

@marcomq marcomq commented Mar 19, 2021

Added ~ and ^ semver semantic to nimble "requires"
Check https://devhints.io/semver for semver examples
#907

@marcomq marcomq changed the title Main Add ~ and ^ semver semantic to nimble "requires" Mar 19, 2021
@marcomq marcomq changed the title Add ~ and ^ semver semantic to nimble "requires" #907 - Add ~ and ^ semver semantic to nimble "requires" Mar 19, 2021
@marcomq marcomq changed the title #907 - Add ~ and ^ semver semantic to nimble "requires" Add ~ and ^ semver semantic to nimble "requires" Mar 19, 2021
Copy link
Collaborator

@dom96 dom96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thank you!

Approving, but I'd like to see some docs for this, can you write some explaining what these new operators mean exactly in the readme? :)

src/nimblepkg/version.nim Outdated Show resolved Hide resolved
src/nimblepkg/version.nim Outdated Show resolved Hide resolved
src/nimblepkg/version.nim Outdated Show resolved Hide resolved
marcomq and others added 2 commits March 20, 2021 13:54
Co-authored-by: Dominik Picheta <[email protected]>
Co-authored-by: Dominik Picheta <[email protected]>
@marcomq
Copy link
Contributor Author

marcomq commented Mar 20, 2021

Thx for the review :) I will update the readme.
After checking the semver documentation again, I have to check if everything is still correct for 2 or 4 digit version numbers. Maybe I also need to add some test cases.

@marcomq
Copy link
Contributor Author

marcomq commented Mar 20, 2021

Sry - after creating some documentation, I made some fixes and refactored a lot of code. However, I think the solution is better than my previous one. Let me know if I should make some additional changes.

Copy link
Collaborator

@dom96 dom96 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, some suggestions for follow up:

  • Create tests that verify the version range created for these new intersections, I think that is a much clearer way to test these :)

Might be possible to simplify getNextIncompatibleVersion, but let's get it merged, we can iterate later. As long as we have tests it's all good :)

@dom96 dom96 merged commit 8eca18a into nim-lang:master Jul 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants