gaelcolas / Sampler

Module template with build pipeline and examples, including DSC elements.
MIT License
172 stars 42 forks source link

Get-BuildVersion uses GitVersion if installed #279

Closed stevehose closed 3 years ago

stevehose commented 3 years ago

The function Get-BuildVersion uses logic that was created a while back. The logic is that if GitVersion is installed then the developer intends to use it. Unfortunately, that isn't necessarily true. Our Azure build pipeline agent has GitVersion installed but we don't intend to use it in the near future. Unfortunately, that means that GitVersion derives a semantic version that is not what we are using.

I noticed that Build_ModuleOutput_ModuleBuilder has a parameter for $ModuleVersion. Unfortunately, if I add that property to the build.yaml, it doesn't pick it up. That looks like the ideal way to provide Get-BuildVersion with the desired value.

Your thoughts?

johlju commented 3 years ago

I think you can add a environment variable ModuleVersion and it should pick it up, or set that property in the parent scope and it should also be picked up. 🤔 I haven't tried it, but if the ModuleVersion is already set if will not call Get-ModuleVersion.

How do you update the module version? Is updating it in build.yaml a preference?

stevehose commented 3 years ago

Yes, a simple update in build.yaml would be great. I admit that I should take the plunge and learn gitversion, but that is not high enough in the priority queue right now. I'll test out the environment variable approach. I'm not sure how I can load the variable in the parent scope. The task has it as a parameter but the parent doesn't. That means I would need to hook into the task running calling process somehow. I'm trying to avoid customizing the tasks.

johlju commented 3 years ago

Try setting the variable in the PS Session before invoking the build task,. If that does not work, try to add a environment variable prior to running the build task. I have not tried either, but should work. Let me know the result. 🙂

"...or obtained using property. The latter gets the value of session or environment variable or the optional default." https://github.com/nightroman/Invoke-Build/wiki/Script-Tutorial#build-properties

Uses property: https://github.com/gaelcolas/Sampler/blob/c7dc73c3d310972ab6a72a6ff7dfe830312eaba2/.build/tasks/Build-Module.ModuleBuilder.build.ps1#L33

johlju commented 3 years ago

Yes, a simple update in build.yaml would be great.

I think this should work, but there is a bug. If you provide SemVer: '999.0.0-preview1 in build.yml it should take precedence, but this line hardcodes the version to ModuleVersion.

https://github.com/gaelcolas/Sampler/blob/c7dc73c3d310972ab6a72a6ff7dfe830312eaba2/.build/tasks/Build-Module.ModuleBuilder.build.ps1#L96

I think we should have the following prior:

if (-not $buildModuleParams.ContainsKey('SemVer'))
{
    $buildModuleParams.Add('SemVer', $ModuleVersion)
}

$BuiltModule = Build-Module @buildModuleParams -PassThru 
johlju commented 3 years ago

Sent in PR https://github.com/gaelcolas/Sampler/pull/282.

stevehose commented 3 years ago

Uh... That was fast! Awesome!!

johlju commented 3 years ago

It was an easy fix 😉 I added how it works here now: https://github.com/gaelcolas/Sampler#moduleversion

A preview will be released shortly so you can test it out.

stevehose commented 3 years ago

@johlju - The documentation is very clear now. Thanks for that! I like the approach. Thank you for your responsiveness also.