Skip to content
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

Test ALL the tooling in CI #1328

Open
porridge opened this issue Feb 3, 2020 · 0 comments
Open

Test ALL the tooling in CI #1328

porridge opened this issue Feb 3, 2020 · 0 comments

Comments

@porridge
Copy link
Member

porridge commented Feb 3, 2020

How does the issue in #1244 manifest?

Code generation script barfs when running with GOPATH unset or GOBIN set to something else than $GOPATH/bin, IIRC.

It isn't detected by the CI, which makes it hard to determine which dependencies can be bumped.

Right. I don't think we invoke code generation in the CI runs at all, do we?

How about adding a replace directive in go.mod instead? This would make it clear that this specific version is intended to be used.

When reading the documentation for go.mod it did not seem to me that this is what replace is to be used for? Also, I'm not sure that:

require k8s.io/code-generator v0.16.6
replace k8s.io/code-generator v0.16.6 => k8s.io/code-generator v0.18.whatever

would qualify as making the intent clear :-)

Not sure if comments can be used in go.mod, but having an additional comment here would be great as well.

I think it allows comments. However I don't think that would scale. Even if we commented that a particular version of a dependency is necessary for some reason X, we would have no idea whether that ceases to be true if X is no longer a valid reason (as there can be other reasons).

This whole file is a delicate mesh of subtleties and it's not possible to document what would fail in case one of these lines are changed. The only assertion we can make is that "it worked like this for whoever touched it last, possibly only after lots of sweat and tears". I think that increasing test coverage to also invoke the less often used tooling is the only way to go...

Originally posted by @porridge in #1326 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant