Allow disabling MinVer

[slang25]

I've noticed that with solutions of many projects, the total MinVer execution time can exceed a second, so thought it would be useful to add an option to disable it when developing locally.

[slang25]

Hmmm, the MYGET_ADAMRALPH_CI_API_KEY secret is not getting passed, is that cause I'm not you?

[adamralph]

@slang25 thanks for raising this!

My first reaction was to see if there's a way to skip a given target using some MSBuild magic but I can't find anything, so I guess something like this is required to allow skipping.

I wonder if the name should be MinVerSkip to align with the rest of the properties?

Hmmm, the MYGET_ADAMRALPH_CI_API_KEY secret is not getting passed, is that cause I'm not you?

Yeah, I think so. I'll see if I can fix that.

[bording]

@adamralph Another option to consider here would be to cache the results to disk, so that the version only needs to be calculated once, which would speed everything up without having to disable MinVer in your projects. Conditionally disabling MinVer could lead to bugs where you've accidentally disabled MinVer when you didn't intend to.

[slang25]

Good idea on the property name, done.

On caching, that's an interesting one. We'd always want to execute git to see if we need to bust a cache, so we wouldn't be putting much of the time saving behind the cache. There could be caching done at a solution level, so that the first git command does the work and the others use the result.

It feels like too many moving parts for the benefit. With the ability to skip, it's something you have to add explicitly, we can't stop people getting it wrong, but there's something to be said for the simplicity of it. I think it's better than at the moment, where you might be tempted to put a condition on the PackageReference, but no realise that if your CI runs the restore in a separate step you don't have a Configuration value, so it could skip when you didn't want it to (ask me how I know 🤦‍♂️, not using MinVer but same scenario).

[bording]

On caching, that's an interesting one. We'd always want to execute git to see if we need to bust a cache, so we wouldn't be putting much of the time saving behind the cache. There could be caching done at a solution level, so that the first git command does the work and the others use the result.

I'm certainly not suggesting that it wouldn't be more complex, but other versioning tools already do something similar, so I think it's something that could be considered here, and I have ideas for how it could be done without needing to run git, so it could end up saving time.

However, this PR isn't the place for going into more detail. I'll open a new issue.

it. I think it's better than at the moment, where you might be tempted to put a condition on the PackageReference, but no realize that if your CI runs the restore in a separate step you don't have a Configuration value, so it could skip when you didn't want it to (ask me how I know 🤦‍♂️, not using MinVer but same scenario).

Yeah, when I first saw this PR, I was thinking "Why not just condition the package reference?" but came to the conclusion that it would be a bad idea to alter the restore state like that! 😄

[slang25]

@bording Ah I understand now, yeah it sounds like a good idea and one for a separate issue but I see why you mention it.

[slang25]

@adamralph Let me know if there's anything you want doing with this, or if it needs further discussion.

[adamralph]

@slang25 sorry for the delay. This is good to go. I'll push out a pre-release ASAP and let you know.

[adamralph]

@slang25 I've pushed this out in 2.1.0-rc.1. Care to give it a try?

[adamralph]

ping @slang25 ☝️

[slang25]

Sorry for not getting back to you @adamralph, it works great! 👍

[adamralph]

Thanks @slang25, good to know!