Skip to content

Conversation

@chris-codeflow
Copy link
Contributor

@chris-codeflow chris-codeflow commented Oct 14, 2021

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.

Related Issue

Fixes #2879

Motivation and Context

The motivation for this PR was to correct the XML documentation for SemanticVersion.ToString (specifically the IFormattable.ToString implementation) and then implement a series of improvements in the general ToString implementation.

How Has This Been Tested?

  • Updated and re-ran the unit tests for the SemanticVersion class.
  • Added a new test to verify that a FormatException exception is thrown when an unknown format is specified.

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

- 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.
- Align SemanticVersion.ToString implementation of IFormattable.ToString with .NET Standard 2.0 definition.
Copy link
Member

@asbjornu asbjornu left a comment

Choose a reason for hiding this comment

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

Looks like a decent improvement, thanks! Can you please fix the failing tests?

@chris-codeflow
Copy link
Contributor Author

@asbjornu . The DotNet Format failure is failing on projects in the build folder which I have not changed in my PR, so I suspect the same failures occurred in the last PR.

@arturcic
Copy link
Member

@chris-codeflow I will have a look and fix the build formatting, then you should be able to rebase

…eTag.ToString

- Align SemanticVersion.ToString implementation of IFormattable.ToString with .NET Standard 2.0 definition.
@arturcic
Copy link
Member

@asbjornu seems like the dotnet format that is part of the .net sdk has changed from .net 6.- rc1 to rc2

@arturcic
Copy link
Member

@chris-codeflow you can now rebase on top of main

- 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.
- Align SemanticVersion.ToString implementation of IFormattable.ToString with .NET Standard 2.0 definition.
…eTag.ToString

- Align SemanticVersion.ToString implementation of IFormattable.ToString with .NET Standard 2.0 definition.
@chris-codeflow
Copy link
Contributor Author

chris-codeflow commented Oct 14, 2021

@arturcic . The rebase onto main has added duplicate commits, as expected. Please do a squash merge for this PR, so we don't pollute the main branch's commit history.

I broke the golden rule of rebasing which is never to rebase a branch that has been published!

@chris-codeflow
Copy link
Contributor Author

chris-codeflow commented Oct 15, 2021

@arturcic and @asbjornu. The Test artifacts in docker (5.0, alpine.3.12-x64) check is still pending, but the step has completed.

@arturcic
Copy link
Member

@arturcic and @asbjornu. The Test artifacts in docker (5.0, alpine.3.12-x64) check is still pending, but the step has completed.

I think GitHub is not displaying the status properly, si if @asbjornu agrees we can merge this

@asbjornu asbjornu merged commit db23e17 into GitTools:main Oct 15, 2021
@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2021

Thank you @chris-codeflow for your contribution!

@chris-codeflow chris-codeflow deleted the feature/Improve-ToString branch October 15, 2021 16:06
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.

[Improvement] SemanticVersion.ToString

3 participants