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

Output the version number parts as properties #109

Closed
adamralph opened this issue Nov 19, 2018 · 7 comments
Closed

Output the version number parts as properties #109

adamralph opened this issue Nov 19, 2018 · 7 comments
Labels
enhancement New feature or request
Milestone

Comments

@adamralph
Copy link
Owner

adamralph commented Nov 19, 2018

More MinVer properties:

  • MinVerMajor
  • MinVerMinor
  • MinVerPatch

These can be used for things like setting the assembly version or assembly file version.

@adamralph adamralph added the enhancement New feature or request label Nov 19, 2018
@adamralph adamralph added this to the 1.0.0 milestone Nov 19, 2018
@adamralph adamralph self-assigned this Nov 19, 2018
@bording
Copy link
Collaborator

bording commented Nov 20, 2018

How do you envision something like {MinVerMajor}.0.0 being used to set the assembly version? An end user wouldn't be able to set that in their project file because the property would be evaluated before the target ran to give MinVerMajor a value. They would have to write a custom target that runs after MinVer but before CoreCompile to be able to do that.

I also don't find a lot of value in setting AssemblyFileVersion to anything other than the Major.Minor.Patch.0, which is what the project system is already doing:

image

I do agree that we should do something that lets you opt in to setting AssemblyVersion to Major.0.0.0, but I'm sure that trying to open that up to full configurability makes sense.

Given that, I'm not sure it really makes a lot of sense to add these properties.

@adamralph
Copy link
Owner Author

Thanks for the comments @bording.

They would have to write a custom target that runs after MinVer but before CoreCompile to be able to do that.

Yep, they would have do that, but I think just after MinVer is enough.


I also don't find a lot of value in setting AssemblyFileVersion to anything other than the Major.Minor.Patch.0

The current guidance says:

CONSIDER including a continuous integration build number as the AssemblyFileVersion revision.


I do agree that we should do something that lets you opt in to setting AssemblyVersion to Major.0.0.0, but I'm sure that trying to open that up to full configurability makes sense.

I guess we could just have an option for just that specific behaviour, or even just make MinVer do this by default, but under the current design, that would still require richer output from MinVer.dll and thus more noise in the build output. If, hypothetically, this were the only new output variable required, I'd still be interested to know if you had a better than idea than what I've currently done regarding the implementation.

@bording
Copy link
Collaborator

bording commented Nov 21, 2018

Yep, they would have do that, but I think just after MinVer is enough.

That is because MinVer also has to be called before CoreCompile, so using just AfterTargets="MinVer" is enough.

The current guidance says:

It does, but it's also just another display value, so I don't know much value there is in setting it that way. For example, if you look at how GitVersion handles it by default, you just get Major.Minor.Patch.0.

However, following the recommendation for AssemblyVersion makes a lot of sense when you are strong-naming the assembly, so I think it's worth doing.

I guess we could just have an option for just that specific behaviour, or even just make MinVer do this by default, but under the current design, that would still require richer output from MinVer.dll and thus more noise in the build output. If, hypothetically, this were the only new output variable required, I'd still be interested to know if you had a better than idea than what I've currently done regarding the implementation.

If we are just trying to get the Major portion of the version, I think we could likely get MSBuild to parse out just that portion of the single version, meaning we wouldn't need to alter the console output.

Another idea could be to go ahead and create a task, but the task just executes the command for now, and then you can do easier parsing of the command-line output in C#, and the task outputs whatever properties you want to have available for the targets to use.

@bording
Copy link
Collaborator

bording commented Nov 21, 2018

After thinking about this a bit more, I do think its reasonable for the following things:

  • Add properties similar to MinVerVersion that MinVer sets for all the components that make up the calculated version: major, minor, patch, prerelease, height, build metadata (though this will be coming from the property the set by the user, so nothing special to do here, right?)
  • The AssemblyFileVersion property should be set in the MinVer target to be major.0.0.0
  • Make it clear in the README that a target with AfterTargets="MinVer" is where you customize any of the assembly attributes you want (including changing the default AssemblyFileVersion MinVer has just set) using all of the new properties that make up the version components.

The thing I don't think we should do is mess around with VersionPrefix or VersionSuffix. If we've already set Version and are providing properties for all of the calculated version values, then there's no reason I can see to set those.

@adamralph
Copy link
Owner Author

Add properties similar to MinVerVersion that MinVer sets for all the components that make up the calculated version: major, minor, patch, prerelease, height, ...

The only thing I'm not 100% sure of is whether prerelease and height should be separated.

...build metadata (though this will be coming from the property the set by the user, so nothing special to do here, right?)

Not necessarily. If the user specifies build metadata in a tag, any build metadata in the option will be appended to it.

The AssemblyFileVersion property should be set in the MinVer target to be major.0.0.0

Agreed. And if anyone wants to change that, they can do it in their own target.

Make it clear in the README that a target with AfterTargets="MinVer" is where you customize any of the assembly attributes you want (including changing the default AssemblyFileVersion MinVer has just set) using all of the new properties that make up the version components.

Agreed. I think it's just a matter of enriching the FAQ.

The thing I don't think we should do is mess around with VersionPrefix or VersionSuffix.

Agreed. Also because there is no way to consistently express build metadata in those properties for both RTM and pre-release versions.

@adamralph
Copy link
Owner Author

adamralph commented Nov 21, 2018

Another idea could be to go ahead and create a task, but the task just executes the command for now, and then you can do easier parsing of the command-line output in C#, and the task outputs whatever properties you want to have available for the targets to use.

Yeah, that's something I've been thinking about too. In fact, I already started to spike it in another branch, just to get nicer errors than when using the Exec task. However, if it's just for that, and just to get rid of a little more noise in the logs before we're able to do #112, I'm not sure it's worth the effort and extra complexity. We'd then need yet another project sitting between MinVer and MinVer.Lib to be the executable.

@adamralph
Copy link
Owner Author

Released in alpha 16.

@adamralph adamralph removed their assignment Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants