Skip to content

Change Int32 to Int64 #3028

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

Merged
merged 1 commit into from
Mar 5, 2022
Merged

Change Int32 to Int64 #3028

merged 1 commit into from
Mar 5, 2022

Conversation

asbjornu
Copy link
Member

@asbjornu asbjornu commented Mar 3, 2022

Change the following properties from Int32 to to Int64 to avoid System.OverflowException on int.Parse() of very large numbers:

  • SemanticVersion.Major
  • SemanticVersion.Minor
  • SemanticVersion.Patch
  • SemanticVersionBuildMetaData.CommitsSinceTag
  • SemanticVersionBuildMetaData.CommitsSinceVersionSource
  • SemanticVersionBuildMetaData.UncommittedChanges
  • SemanticVersionPreReleaseTag.Number

Related Issue

Fixes #2715.
Fixes #2915.
Fixes #2390.

Motivation and Context

Large numbers in a version number may cause the following lines to throw a System.OverflowException:

Major = int.Parse(parsed.Groups["Major"].Value),
Minor = parsed.Groups["Minor"].Success ? int.Parse(parsed.Groups["Minor"].Value) : 0,
Patch = parsed.Groups["Patch"].Success ? int.Parse(parsed.Groups["Patch"].Value) : 0,

semanticVersionBuildMetaData.CommitsSinceTag = int.Parse(parsed.Groups["BuildNumber"].Value);

var number = match.Groups["number"].Success ? int.Parse(match.Groups["number"].Value) : (int?)null;

How Has This Been Tested?

I've added test cases that failed before the code changes were implemented.

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.

@asbjornu asbjornu requested a review from arturcic March 3, 2022 20:01
@arturcic
Copy link
Member

arturcic commented Mar 3, 2022

@asbjornu is this a breaking change?

@asbjornu
Copy link
Member Author

asbjornu commented Mar 3, 2022

@asbjornu is this a breaking change?

I shouldn't think so, except perhaps if someone consumes GitVersion.Core.dll as a library. But we decided years ago that such use was unsupported and would not affect SemVer concerns.

@arturcic
Copy link
Member

arturcic commented Mar 4, 2022

Please rebase and we can merge then

Change the following properties from `Int32` to to `Int64` to avoid
`System.OverflowException` on `int.Parse()` of very large numbers:

- `SemanticVersion.Major`
- `SemanticVersion.Minor`
- `SemanticVersion.Patch`
- `SemanticVersionBuildMetaData.CommitsSinceTag`
- `SemanticVersionBuildMetaData.CommitsSinceVersionSource`
- `SemanticVersionBuildMetaData.UncommittedChanges`
- `SemanticVersionPreReleaseTag.Number`
@asbjornu asbjornu enabled auto-merge March 4, 2022 23:54
@asbjornu asbjornu merged commit 19462e1 into GitTools:main Mar 5, 2022
@mergify
Copy link
Contributor

mergify bot commented Mar 5, 2022

Thank you @asbjornu for your contribution!

@asbjornu asbjornu deleted the feature/int64 branch November 3, 2022 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants