search

ScoopInstaller / GithubActions

Github Actions for Scoop buckets
MIT License
24 stars 22 forks source link

feat: Support checkver's option "THROW_ERROR" #5

Closed CrendKing closed 2 years ago

CrendKing commented 2 years ago

For issue https://github.com/ScoopInstaller/Scoop/issues/4863.

Other than simply passing along the new option to underlying script, I noticed that the environment variables from GitHub Action yaml's env section are always converted to strings. For example, I tested the following in excavator.yml:

foo: 0
foo: 1
foo: '0'
foo: '1'
foo: true
foo: false
foo: 'true'
foo: 'false'

For all these, [bool] $env.foo are always true and ($env.foo).GetType() are always System.String. So the existing [bool] $env:SKIP_UPDATED is always true. Not very impacting since most people use '1' anyways. But if we want to default the new THROW_ERROR to false, I think it's better to fix this and make it consistent. There are ways to convert string to boolean in PowerShell in more complete way, but I think checking equalization to '1' is probably good enough. Let me know if this needs change.

rashil2000 commented 2 years ago

Nice observation!!

rashil2000 commented 2 years ago

We'll have to revert this PR for now. The changes in Scoop core haven't been released yet, and as a result all the Excavator runs are failing with this error.

image

/cc @issaclin32 @CrendKing we should wait for Scoop core release and then merge again.

CrendKing commented 2 years ago

Sorry. I forgot to mention this. Alternatively, we could change auto-pr to use the splat form to pass param (i.e. Add "ThrowError" to the $param HashTable only if the value is $true). Do you want that? I could do it.

rashil2000 commented 2 years ago

Changes to auto-pr.ps1 would again happen in the develop branch, right? We would still have to wait for a release.

CrendKing commented 2 years ago

Of course. If you think this kind of changes happen rarely, and we don't need to bother a future-proof approach, then let's wait.

rashil2000 commented 2 years ago

Instead, can you do something like this: https://github.com/ScoopInstaller/GithubActions/blob/24f10081a22578c2a378e01ead79945e6a1a8ed9/src/Action/Scheduled.psm1#L24

CrendKing commented 2 years ago

To auto-pr.ps1?

rashil2000 commented 2 years ago

No, to GithubActions/src/Action/Scheduled.psm1

CrendKing commented 2 years ago

https://github.com/ScoopInstaller/GithubActions/pull/6