Support 32-bit architecture#57
Support 32-bit architecture#57emgre wants to merge 1 commit intoactions:masteractions/setup-java:masterfrom
Conversation
6cd3630 to
8b062ac
Compare
|
|
||
| let archExtension = ''; | ||
| if (arch === 'x86') { | ||
| archExtension = 'i686'; |
There was a problem hiding this comment.
Where did this extension come from? Is it not x86?
There was a problem hiding this comment.
i686 is the latest iteration of the IA-32 (commonly known as x86). From today's perspective, it's a synonym of x86 (although it is technically not). I didn't choose this extension, I just looked at https://cdn.azul.com/zulu/bin/ and saw that's what they use.
| } else if (arch === 'x64') { | ||
| archExtension = 'x64'; | ||
| } else { | ||
| throw new Error(`architecture "${arch}" is not int [x86 | x64]`); |
There was a problem hiding this comment.
We should validate that the architecture input is valid when we retrieve the input in https://github.com/actions/setup-java/blob/master/src/setup-java.ts#L12
Then this whole block can just be:
const archExtension = arch === 'x86' ? 'i686' : 'x64';
There was a problem hiding this comment.
Indeed. I'm not very familiar with the GitHub Actions core library (and JS in general) so bear with me. I'll make the change soon.
There was a problem hiding this comment.
No worries! Let me know if you need any help
There was a problem hiding this comment.
Should be "not in" instead of "not int".
|
Hi folks. Any progress updates on this PR? |
|
@konradpabjan This PR can now be closed. |
|
Replaced with #95 |
Fix #55
I'm using this code in this GitHub Actions pipeline: https://github.com/dnp3/opendnp3/actions
I only tried it on Windows. Perhaps some integration tests could be added to verify that the proper version is fetched. I'm not sure to understand how your tests are run, so I didn't do it.