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

chore: Set versions on dll #603

Closed
wants to merge 1 commit into from
Closed

chore: Set versions on dll #603

wants to merge 1 commit into from

Conversation

mikkelbu
Copy link
Member

No description provided.

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.

Besides the nitpick, a simple solution.

@@ -145,7 +145,9 @@ Task("Build")
NoRestore = true,
MSBuildSettings = new DotNetMSBuildSettings
{
Version = packageVersion
Version = packageVersion,
FileVersion = packageVersion,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
FileVersion = packageVersion,
FileVersion = packageVersion,

It doesn't show in github, but there is a TAB between FileVersion and the = token.

@mikkelbu
Copy link
Member Author

Besides the nitpick, a simple solution.

The problem is that it does work for some reason :(. I sets version in the dlls when I run it locally, but not when appveyor runs the cake script, so I need to experiment some more.

@mikkelbu mikkelbu marked this pull request as draft September 27, 2023 09:49
@manfred-brands
Copy link
Member

Do we even need the AppVeyor build?
We could upload artifacts from GitHub or even upload to GitHub packages.
It would require changing versioning. I tried dotnet cake for my nunit PR but had to resort to using git describe and tags.

@OsirisTerje
Copy link
Member

OsirisTerje commented Sep 27, 2023

We should fix this across all systems.

  • The adapter gets its version through the cake script, but not the publishing, except I have the appveyor build that publish to myget (still works after the downtime last month). I do manual builds and uploads to nuget, in order to control the exact versioning. Still using cake though.
  • NUnit itself deploys more or less the same way. Versioning through cake.
  • The console/engine uses GitVersion and have automated the deployment to nuget and chocolatey (!), also automated the releases to github. Versioning is great, but it takes me some time each time to just remember the steps and the requirements to make the automation work. But, it is actually great.

I don't think we should mess up the versioning too much. Either GitVersion or the cake, and not using git describe and tags (I understand the frustration though :-) ) .

The good thing about the appveyor build is that it actually deploys to MyGet for every main/master build, so that constitutes the fastest development builds we can have, and it works (as long as Myget works). So I dont think we should take those down before we have the final solution out.

Imho, I believe the GitVersion is the way to go. Not sure if we should automate as much as the engine/console - meaning, the requirements there are rather strict, the automation in itself i very good. Example, you have to indicate which issues should be part of the release (yeah, that is good), they have to have at least one of a given set of labels , they have to be closed. It does have a release notes feature, but I havent managed to get that to work. I believe it was Charlie, and possibly Chris, who have done this system, and there are multiple scripts there, so that is what makes it hard.

See https://github.com/nunit/nunit-console/wiki/Creating-a-New-Release

@manfred-brands
Copy link
Member

I agree consistency between the repositories would be beneficial although I thought they were previously deliberately made independent.

Do we need AppVeyor to push to myget? Can that not be done using cake or GitHub action with NuGet push?

@OsirisTerje
Copy link
Member

We absolutely can push to Myget without appveyor. There is a key needed, and the rest is standard. But is the appveyor really the problem here?

Btw, the appveyor uses the cake script too to push it up, like this PushNuGetPackage(PACKAGE_DIR + package.PackageFileName, settings.MyGetApiKey, MYGET_PUSH_URL); I think the method there is a Cake built-in.

So to replace it, we just need the keys (they seems to be listed in the appveyor file, but they might not be the current ones, I hope. The call to cake (or build.ps) can be done from any script.

@manfred-brands
Copy link
Member

I just wondered why we need two builds: GitHub action and AppVeyor. The first is more integrated with GitHub. So pure for simplicity I would drop the latter. Doing it twice would be twice the cost, which might be not be even relevant for an open source project.

@OsirisTerje
Copy link
Member

OsirisTerje commented Sep 27, 2023

We don't need two, but the github action build currently doesn't push to Myget, so we would have to add that. And fix up so it gets the keys. As long as Myget works, I think it is nice to push the dev packages there, Github packages have the credential issue, you have to be logged in to use it.

I absolutely have no problems with getting rid of the appveyor build itself. If you or some one have the time, please just go on!

@mikkelbu
Copy link
Member Author

In the analyzers repo we (currently) use AppVeyor to do the build artifacts that we release. The GitHub action were offered by a contributor (the only contribution to this repo AFAICR), so before we start removing AppVeyor we need to have the same functionality in the GitHub actions.

Relating to this PR, then I'm unsure if the problem is AppVeyor or Cake, but I'll try to add some more logging to the build (but I don't think I'll have time until the weekend due to several deadlines at work).

@mikkelbu
Copy link
Member Author

Moved the removal of AppVeyor to #681 and closing this (was solved in #680)

@mikkelbu mikkelbu closed this Jan 27, 2024
@mikkelbu mikkelbu deleted the chore/set-versions- branch January 27, 2024 20:44
@mikkelbu mikkelbu added this to the Closed Without Action milestone Jan 27, 2024
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.

3 participants