Add file version & copyright info to windows package#13506
Conversation
…adata Agent-Logs-Url: https://github.com/kilasuit/cli/sessions/2fa3166e-24be-468c-a3be-a2b14932712f Co-authored-by: kilasuit <6355225+kilasuit@users.noreply.github.com>
Agent-Logs-Url: https://github.com/kilasuit/cli/sessions/2fa3166e-24be-468c-a3be-a2b14932712f Co-authored-by: kilasuit <6355225+kilasuit@users.noreply.github.com>
|
Thanks for your pull request! Unfortunately, it doesn't meet the requirements for review:
Please update your PR to address the above. This PR will be automatically closed in 4 days if these requirements are not met. Full contribution requirements
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds guardrails around Windows resource/version metadata by enforcing a non-empty legal copyright string and ensuring the winres generation script passes explicit version components to goversioninfo.
Changes:
- Added Go tests validating
versioninfo.template.jsonand presence of fixed version flags ingen-winres.ps1. - Populated
LegalCopyrightinversioninfo.template.json. - Updated
gen-winres.ps1to parse product version components and pass-ver-*/-product-ver-*flags togoversioninfo.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| script/winres_test.go | Adds tests to assert version template and winres script contain required metadata/flags |
| script/versioninfo.template.json | Sets a non-empty LegalCopyright value |
| script/gen-winres.ps1 | Parses version components and passes fixed version fields to goversioninfo |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| $_version_major = [int]$_product_version_components.Groups[1].Value | ||
| $_version_minor = [int]$_product_version_components.Groups[2].Value | ||
| $_version_patch = [int]$_product_version_components.Groups[3].Value | ||
| $_version_build = 0 | ||
| if (-not [string]::IsNullOrEmpty($_product_version_components.Groups[4].Value)) { | ||
| $_version_build = [int]$_product_version_components.Groups[4].Value |
|
|
||
| $_product_version_components = [regex]::Match($_product_version, "^[^0-9]*([0-9]+)\.([0-9]+)\.([0-9]+)(?:\.([0-9]+))?") | ||
| if (-not $_product_version_components.Success) { | ||
| Write-Host "error: product-version argument '$_product_version' must include semantic version numbers" |
There was a problem hiding this comment.
Good point. We should add an example. Also the input must not include the v prefix.
|
@babakks I am not gonna be able to spend any more time back and forth on this so i hope that this is good enough start |
babakks
left a comment
There was a problem hiding this comment.
Thanks for the PR! I haven't yet tried it, and here are some comments.
I'll check this out later and if no progress I'll wrap it up. So, no worries.
| $_version_build = 0 | ||
| if (-not [string]::IsNullOrEmpty($_product_version_components.Groups[4].Value)) { | ||
| $_version_build = [int]$_product_version_components.Groups[4].Value |
There was a problem hiding this comment.
We don't have build number in our version, so let's get rid of these and populate the flags with zero.
| @@ -0,0 +1,43 @@ | |||
| package main | |||
There was a problem hiding this comment.
This is not a reliable test. Let's remove it all.
fixes #13079