-
Notifications
You must be signed in to change notification settings - Fork 150
pkg/console/controllers/clidownloads/controller: Arch qualifiers for Mac/Windows #353
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pkg/console/controllers/clidownloads/controller: Arch qualifiers for Mac/Windows #353
Conversation
da6f516 to
c209e6d
Compare
…Mac/Windows Folks could presumably be running these OSes on other architectures. For example, the GOOS/GOARCH docs [1] provide for darwin on 386, amd64, arm, and arm64 and windows on 386 and amd64 (although Windows runs on ARM64 [2] and possibly other architectures as well). I'm sticking with x86_64 for consistency with the Linux entry from 865d84d (Enable multi-arch Linux links in clidownloads, 2019-11-08, openshift#343), although Linus says the x86-64 form predates the underscore form [3]. [1]: https://golang.org/doc/install/source#environment [2]: https://docs.microsoft.com/en-us/windows/arm/ [3]: https://lkml.org/lkml/2004/2/21/110
c209e6d to
910af2c
Compare
|
/lgtm Works for me. I think both @wking / @yselkowitz have stronger opinions about the link names than I do. These are sufficiently clear. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: benjaminapetersen, wking The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
These names should be those expected by users of those platforms. One does not generally say "Windows for x86_64", and Windows for ARM is rare enough that it needs to be called out as such and would not be confused (not to mention that golang has no support for it right now). The only versions of Mac currently supported are 64-bit Intel. All versions of Mac for 32-bit Intel are no longer supported by Apple, and this would not be confused with iOS (which is also included in "darwin") which runs on ARM. Therefore there is no need to qualify Mac. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
2 similar comments
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
Folks could presumably be running these OSes on other architectures. For example, the GOOS/GOARCH docs provide for darwin on 386, amd64, arm, and arm64 and windows on 386 and amd64 (although Windows runs on ARM64 and possibly other architectures as well).
I'm sticking with
x86_64for consistency with the Linux entry from 865d84d (#343). Previous discussion starting here. CC @yselkowitz.