Disable version check for external libraries #1938
Conversation
There was a problem hiding this comment.
Thanks for this @leaanthony.
Just a thought before this is merged. I don't think we should require users of our library to set a DisableVersionCheck flag to make Task work as package. I feel like this should be the default behaviour and our CLI entrypoint should enable it instead (basically, the reverse of this PR).
|
I actually agree with you. This got me over the line but yeah adding the in the CLI and not |
leaanthony
changed the title
|
@pd93 - Updated the PR to run the version initialisation on cli startup. Tidied the version output if there is no sum. |
| if sum == "" { | ||
| sum = info.Main.Sum | ||
| } | ||
| return | ||
| } |
There was a problem hiding this comment.
We need to keep these empty checks as these variables are pre-populated by ldflags in some package managers. The default value also needs to be empty for the same reason.
There have been quite a few accidental changes around this function. We should probably add some comments here to explain this too.
There was a problem hiding this comment.
Coming at it fresh it does seem a bit strange 😅 The reason for the change was a failing test that assumed it would be "unknown" but would return blank. What's the best approach to fix that? Set it in Init if it's not set in the buildinfo?
There was a problem hiding this comment.
This is because our tests treat Task as a package, not a CLI. This is a bit awkward to fix, but I think we could add a setting in the executor called EnableVersionCheck (Similar to the DisableVersionCheck you had previously). This setting would then need to be added to the CLI entrypoint and any relevant tests that need the version check functionality.
This PR adds an internal flag to the executor to disable version checks. This is to ensure that any application that uses Task as a library doesn't error when Task checks the buildinfo.
This PR also renames
closetocloserintask.goas it clashes with the keyword `close.