-
Notifications
You must be signed in to change notification settings - Fork 141
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
host-specific and platform overlapped in validation #414
Comments
Fix #414 |
fixed, close |
On Thu, Jul 06, 2017 at 02:58:39AM +0000, 梁辰晔 (Liang Chenye) wrote:
In validate.go, we have both 'host-specific' and 'platform', I think
we should overwrite the platform when we have 'host-specific'
option.
Can we re-open this? I'd rather have logic like:
a. If --platform is set…
a. If --host-specific is set…
a. If the platform value matches the host, proceed with
host-specific validation.
b. If the platform value doesn't match the host, generate a
usage error (a “we can't do that on this host” error, not a
“that config is invalid” error).
b. If --host-specific is not set, run the host-independent checks
for that platform.
b. If --platform is not set, default to the current host platform.
a. If --host-specific is set, proceed with host-specific
validation.
b. If --host-specific is not set, run the host-independent checks
for that platform.
The difference is that with #415, we're using the a.a.a approach for
both a.a.a and a.a.b.
And we're currently assuming ‘linux’ for --platform in b.b instead of
my proposal, which is less convenient for non-Linux users (I expect
validating Linux configs using non-Linux hosts to be less common than
validating same-platform configs on non-Linux hosts).
|
+1 for add a 'warning' to fix a.a.b. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While reviewing #187,
I checked both generate and validate logic.
In validate.go, we have both 'host-specific' and 'platform',
I think we should overwrite the platform when we have 'host-specific' option.
The text was updated successfully, but these errors were encountered: