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

[Improvement] SemanticVersion.ToString #2879

Closed
chris-codeflow opened this issue Oct 14, 2021 · 1 comment · Fixed by #2880
Closed

[Improvement] SemanticVersion.ToString #2879

chris-codeflow opened this issue Oct 14, 2021 · 1 comment · Fixed by #2880
Milestone

Comments

@chris-codeflow
Copy link
Contributor

I noticed in the XML documentation (in the source code) for the SemanticVersion.ToString method (the IFormattable.ToString implementation) that the format pattern for the default SemVer format "s" was incorrect (it included the build metadata). After reviewing the code, I have identified a set of improvements that can be made to the SemanticVersion.ToString implementation.

Detailed Description

  • Correct the SemanticVersion XML comment for the default SemVer ("s") format.
  • Align the IFormattable.ToString method signature with the .NET Standard 2.0 implementation.
  • Throw FormatException exceptions from ToString instead of ArgumentException.
  • Refactor all the ToString tests to use NUnit TestCase attributes.

Context

I needed to understand the difference between the SemVer and FullSemVer output variables, so I checked the source code. When reviewing the code, I identified that the XML documentation for the SemanticVersion.ToString method was incorrect and my review then lead me to identify a number of improvements.

Possible Implementation

See the Detailed Description section above.

@asbjornu asbjornu added this to the 5.x milestone Oct 15, 2021
@arturcic arturcic modified the milestones: 5.x, 5.8.0 Oct 15, 2021
@github-actions
Copy link

🎉 This issue has been resolved in version 5.8.0 🎉
The release is available on:

Your GitReleaseManager bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants