Refactor how/where ko:// is handled. #153
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change more or less completely changes how `ko://` is handled internally to `ko`, but the user-facing changes should only be net-positive. `ko://` was previously stripped at the highest level, and the build logic was unaware, which had some undesirable diagnostic/functional implications that are collectively addressed in this change. With this change, the `ko://` prefix is preserved and passed to the build logic, which internally parses a new `reference` type (this was useful to have Go's type checker find all of the places that needed fixing). The main functional differences are: 1. If a reference is prefixed with `ko://` we will now fail fast in `IsSupportedReference` regardless of whether `--strict` is passed. 2. If a reference is prefixed with `ko://` it will bypass the prefix check, which allows the use of `ko://github.com/another/repo` that references a vendored binary package. For `2.` the absence of the module prefix causes the filtering logic Jon introduced to avoid the reference. This was critical for efficiency when `ko://` isn't around because we feed every string in the yaml through it, but when the user has explicitly decorated things it's the perfect thing to be sensitive to. Fixes: ko-build#146 Fixes: ko-build#152
imjasonh
approved these changes
Apr 30, 2020
albertomilan
added a commit
to albertomilan/ko
that referenced
this pull request
Apr 30, 2020
Refactor how/where ko:// is handled. (ko-build#153)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change more or less completely changes how
ko://is handled internally toko, but the user-facing changes should only be net-positive.ko://was previously stripped at the highest level, and the build logic was unaware, which had some undesirable diagnostic/functional implications that are collectively addressed in this change.With this change, the
ko://prefix is preserved and passed to the build logic, which internally parses a newreferencetype (this was useful to have Go's type checker find all of the places that needed fixing). The main functional differences are:ko://we will now fail fast inIsSupportedReferenceregardless of whether--strictis passed.ko://it will bypass the prefix check, which allows the use ofko://github.com/another/repothat references a vendored binary package.For
2.the absence of the module prefix causes the filtering logic Jon introduced to avoid the reference. This was critical for efficiency whenko://isn't around because we feed every string in the yaml through it, but when the user has explicitly decorated things it's the perfect thing to be sensitive to.Fixes: #146
Fixes: #152
cc @dprotaso