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

Fix SemVerHelper #848

Closed
wants to merge 2 commits into from
Closed

Fix SemVerHelper #848

wants to merge 2 commits into from

Conversation

omares
Copy link

@omares omares commented Jul 1, 2015

Fix for #847

  • Added active pattern for semver matching
  • Fixed parsing by utilizing SemVer active pattern
  • Fixed ToString conversion

@omares
Copy link
Author

omares commented Jul 1, 2015

Update:

  • Added PreRelease parsing according to SemVer specification
  • Removed Build from PreRelease type as it is not needed
  • Fixed Tests

As mentioned in chat i do not know why the when_parsing_many_pre_release_versions test fails, as it is not comprehensible to me. Imho the expectations are not correct.

@forki
Copy link
Member

forki commented Jul 25, 2015

will look into this on monday. thx a lot

@omares
Copy link
Author

omares commented Jul 26, 2015

@forki as mentioned in chat i would be willing to finish this PR when its clear how to proceed :) the helper still includes a small issue i will fix on tuesday.

@matthid
Copy link
Member

matthid commented Apr 25, 2017

Can you try to send the PR on top of my matthid:coreclr branch? This would make sure your changes are not lost in the new netcore release (and you could use them right away from the AppVeyor nuget feed (https://ci.appveyor.com/nuget/fake). If you need help, let me know.

Thanks!

@matthid
Copy link
Member

matthid commented May 6, 2017

There is also another problem with this. nuget.org is still slowly migrating to semver v2 and this change might break users of nuget packages published with FAKE... However I still think this is the correct fix. We probably apply this and look where things break...
It would probably be a good idea to provide the old behavior via an obsolete member.

@matthid matthid self-assigned this May 6, 2017
@matthid matthid added the bug label May 6, 2017
@matthid matthid mentioned this pull request May 6, 2017
@matthid matthid added the waiting for author Some information or action was requested and it needs to be addressed. Or a response from author label May 28, 2017
@matthid
Copy link
Member

matthid commented May 28, 2017

This probably needs some re-basing. Note that I would be much more comfortable with adding this to the new Fake.Core.SemVer package instead of the legacy fake4 code (considering potential breakages).
Maybe related fsprojects/Paket#2319

@matthid
Copy link
Member

matthid commented Jul 26, 2017

Leaving open because of the reported bug.

@matthid matthid removed the waiting for author Some information or action was requested and it needs to be addressed. Or a response from author label Jul 26, 2017
matthid added a commit that referenced this pull request Feb 4, 2018
@matthid
Copy link
Member

matthid commented Feb 4, 2018

I applied this patch to Fake.Core.SemVer, which is the new FAKE 5 module. This removes the headache of maybe introducing a breaking change. Therefore I close this due to inactivity. Thanks for the submission!

@matthid matthid closed this Feb 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants