Conversation
| // | ||
| const versions = core.getMultilineInput('dotnet-version'); | ||
| const installedDotnetVersions: string[] = []; | ||
| let architecture = core.getInput('architecture'); |
There was a problem hiding this comment.
| let architecture = core.getInput('architecture'); | |
| const architecture = core.getInput('architecture'); |
| if (!architecture) { | ||
| architecture = ''; | ||
| } |
There was a problem hiding this comment.
I'm not sure that we need this check here. If the architecture input is not supplied function core.getInput() will return an empty string by default.
| constructor( | ||
| version: string, | ||
| quality: QualityOptions, | ||
| architecture: string = '' |
There was a problem hiding this comment.
| architecture: string = '' | |
| architecture: string |
Do we need a default value here? As I wrote in another comment, the default value for the empty input will be an empty string.
| this.setQuality(dotnetVersion, scriptArguments); | ||
| } | ||
|
|
||
| if (this.architecture != '') { |
There was a problem hiding this comment.
| if (this.architecture != '') { | |
| if (this.architecture) { |
|
|
||
| if (this.architecture != '') { | ||
| scriptArguments.push('--architecture', this.architecture); | ||
| } |
There was a problem hiding this comment.
I suggest adding the same code for Windows.
| architecture: | ||
| description: 'Optional architecture to use. If not provided, will default to the OS architecture.' | ||
| required: False |
There was a problem hiding this comment.
| architecture: | |
| description: 'Optional architecture to use. If not provided, will default to the OS architecture.' | |
| required: False | |
| architecture: | |
| description: 'Optional architecture of the .NET binaries to install. Possible values: amd64, x64, x86, arm64, arm, and s390x, if not provided, defaults to the OS architecture.' |
|
Hi, @kevingosse 👋 ! Thank you for the PR, I left some comments below and I'd also suggest making changes to the documentation. |
|
Just a gentle ping, @kevingosse |
|
Review to be implementation |
|
Hi, @kevingosse, just a gentle ping 📟 |
|
Hey. Sorry I don't have time to work on this anymore. Happy to leave somebody else take over the work. I can close the PR if it creates noise. |
|
Hi, @kevingosse 👋 Thanks for the information! No worries, your PR doesn't create any noise. Let it be here, until we can proceed with it. |
|
Why is that still open? We need setting architecture and DOTNET_ROOT(x86) environment variable, please. |
7 participants
Description:
Add an optional
architectureparameter to set the target architecture (x86, x64, arm64, ...).Related issue:
#72
Check list: