kazel K8sOpenAPIGen should respect SkippedPaths too #32
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
k8s-ci-robot
added
the
cncf-cla: yes
ixdy
changed the title
|
ok, tested. pretty sure this works now. |
spxtr
reviewed
Aug 14, 2017
kazel/generator.go
Outdated
| @@ -52,6 +52,11 @@ func (v *Vendorer) walkGenerated() error { | |||
| // findOpenAPI searches for all packages under root that request OpenAPI. It | |||
| // returns the go import paths. It does not follow symlinks. | |||
| func (v *Vendorer) findOpenAPI(root string) ([]string, error) { | |||
| for _, r := range v.skippedPaths { | |||
| if r.Match([]byte(root)) { | |||
There was a problem hiding this comment.
done. fixed similar calls elsewhere, too.
spxtr
reviewed
Aug 14, 2017
| @@ -42,7 +42,7 @@ func (v *Vendorer) walkGenerated() error { | |||
| return nil | |||
| } | |||
| v.managedAttrs = append(v.managedAttrs, "openapi_targets", "vendor_targets") | |||
| paths, err := v.findOpenAPI(v.root) | |||
| paths, err := v.findOpenAPI(".") | |||
There was a problem hiding this comment.
SkippedPaths uses relative paths, so the regexes won't match unless we use a relative path consistently. We use relative paths (i.e. .) for both walk() and walkSource(), so this was the odd one out.
There was a problem hiding this comment.
(and we chdir to v.root so . will be valid)
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
tested, seems to still work. |
k8s-github-robot
pushed a commit
to kubernetes/kubernetes
that referenced
this pull request
Aug 17, 2017
Automatic merge from submit-queue (batch tested with PRs 49869, 47987, 50211, 50804, 50583) Bump repo-infra/kazel dependency **What this PR does / why we need it**: `kazel` shouldn't be looking under skipped paths (like `_output`) for openapi files. This was fixed in kubernetes/repo-infra#32 and now should be included here. I've tested locally that this now ignores everything under `_output`. **Release note**: ```release-note NONE ``` /assign @mikedanese @spxtr
spxtr
pushed a commit
to spxtr/repo-infra
that referenced
this pull request
Aug 21, 2017
update README to rST format
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.
Some of the non-Bazel kubernetes build scripts toss Go source under
_output/dockerized/go; we should not go looking for OpenAPI packages there.Still testing this (trying to figure out what set of steps actually copies code into
_output/dockerized/gobut I think this will work./assign @mikedanese @spxtr
cc @thockin @nikhita @sttts