-
Notifications
You must be signed in to change notification settings - Fork 723
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
Conversation
…ed csprojs and assemblyinfo.cs files accordingly.
…o fileversionissue
There was a problem hiding this 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
src/NUnitFramework/nunit.framework.legacy/nunit.framework.legacy.csproj
Outdated
Show resolved
Hide resolved
Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>
…cy.csproj Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>
Co-authored-by: Manfred Brands <manfred-brands@users.noreply.github.com>
…o fileversionissue
…iguration .NET 7.0 is out of support as of May 14th 2024.
There was a problem hiding this 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still some issues.
There was a problem hiding this 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
:
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:
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.
95ce9e4
to
3eb3e00
Compare
…raised a PR there too, but the warning can be safely ignored for now
Fixes #4705