Use "" for "unspecified OS", tweak comments #196
Conversation
The fact that `://dummy` is not a valid URL syntax was bothering me, let's support (and default to) the empty string to mean unset. Use a common function for handling this, and also change the comments to clarify that we're not today "working around" the installer, it's simply not specified.
openshift-ci-robot
added
the
size/S
|
/lgtm
Aren't we, though? The goal is for the installer to eventually specify this in the initial config (see openshift/installer#281 (comment)). At which point, we shouldn't expect the |
openshift-ci-robot
added
lgtm
Probably but...I think we should still support not specifying it (for debugging at least). |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashcrow, cgwalters, jlebon The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test e2e-aws |
1 similar comment
|
/test e2e-aws |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
5 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. |
|
/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. |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test e2e-aws |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
/test e2e-aws |
5 similar comments
|
/test e2e-aws |
|
/test e2e-aws |
|
/test e2e-aws |
|
/test e2e-aws |
|
/test e2e-aws |
|
/test e2e-aws |
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
The fact that
://dummyis not a valid URL syntax was bothering me,let's support (and default to) the empty string to mean unset.
Use a common function for handling this, and also change the comments
to clarify that we're not today "working around" the installer, it's
simply not specified.