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

Fixes Issue #4705 - Wrong version numbers in dlls #4706

Merged
merged 27 commits into from May 26, 2024
Merged

Conversation

OsirisTerje
Copy link
Member

Fixes #4705

src/AssemblyInfo.g.cs Outdated Show resolved Hide resolved
Copy link
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

@OsirisTerje Sorry for the delay, I was off sick and couldn't concentrate enough to do reviews.
I also like code generation, but I do think your second option is nicer.
I think we can remove a lot of conditionals in the other projects by utilizing the AssemblyConfiguration set in the Directroy.Build.props

build.cake Outdated Show resolved Hide resolved
src/NUnitFramework/Directory.Build.props Outdated Show resolved Hide resolved
src/NUnitFramework/Directory.Build.props Outdated Show resolved Hide resolved
src/NUnitFramework/Directory.Build.props Outdated Show resolved Hide resolved
src/NUnitFramework/framework/nunit.framework.csproj Outdated Show resolved Hide resolved
src/NUnitFramework/nunitlite/nunitlite.csproj Outdated Show resolved Hide resolved
@OsirisTerje OsirisTerje changed the title Fixes Issue #4705 Fixes Issue #4705 - Wrong version numbers in dlls May 13, 2024
Copy link
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

You added various unnecessary null suppression operators (!).
All of them are unnecessary as NUnit.Analyzer does the suppressing for us as it knows those cannot be null.

Copy link
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

Still some issues.

CakeScripts.Tests/Usings.cs Show resolved Hide resolved
src/NUnitFramework/tests/CakeTests.cs Outdated Show resolved Hide resolved
src/NUnitFramework/tests/WorkItemTests.cs Outdated Show resolved Hide resolved
src/NUnitFramework/TestBuilder.cs Outdated Show resolved Hide resolved
CakeScripts.Tests/VersionParsersTests.cs Outdated Show resolved Hide resolved
Copy link
Member

@manfred-brands manfred-brands left a comment

Choose a reason for hiding this comment

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

I raised one issue in the build.cake script.
It was mentioned originally in my first review but got lost in the further discussions.
I pushed up a commit for that.

One thing though is that if I build this locally it build version: 4.0.0-alpha.88
So maybe the code relies on annotated tags instead of normal ones.
git describe returns 4.0.0-alpha-244-ga604a39ca

I do see confusing tags in the repository some have a v prefix others don't

4.0.0-alpha
4.1.0
4.1.0-alpha.0
v4.0.0
v4.0.0-beta.1
v4.0.1

Note that the CI build doesn't fetch any tags and creates version 0.0.0.0:

image

I added a commit to fetch the tags to see what it does. You can revert this as it isn't needed for CI builds only for publications.
It seems to create 4.2.0 versions:

image

MinVer must pick up the 4.1.0 tag and create the next release.
There are various properties for MinVer we can use to tailor this.

CakeScripts.Tests/CakeScripts.Tests.csproj Show resolved Hide resolved
CakeScripts/VersionParsers.cs Outdated Show resolved Hide resolved
build.cake Outdated Show resolved Hide resolved
build.cake Outdated Show resolved Hide resolved
OsirisTerje and others added 2 commits May 25, 2024 11:22
@OsirisTerje OsirisTerje merged commit 2d20f5d into master May 26, 2024
5 checks passed
@OsirisTerje OsirisTerje deleted the fileversionissue branch May 26, 2024 19:47
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.

The dll's in the release 4.1 has version 4.0.1
4 participants