Skip to content

Rewrite the thanos-bucket component without ksonnet #152

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

Merged
merged 1 commit into from
Oct 21, 2020

Conversation

metalmatze
Copy link
Contributor

As you can see there are no changes in the examples/all so the output
generated is equal to before.

@metalmatze metalmatze requested review from kakkoyun and brancz October 21, 2020 13:30
Copy link
Collaborator

@kakkoyun kakkoyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇 LGTM.

We just need to remove the import line, I believe.
local k = import 'ksonnet/ksonnet.beta.4/k.libsonnet';

@metalmatze
Copy link
Contributor Author

Yes, lol 😄

As you can see there are no changes in the examples/all so the output
generated is equal to before.

Signed-off-by: Matthias Loibl <[email protected]>
@metalmatze metalmatze force-pushed the no-ksonnet-thanos-bucket branch from 4465de7 to 033cdab Compare October 21, 2020 15:57
@kakkoyun kakkoyun merged commit a05a421 into thanos-io:master Oct 21, 2020
@metalmatze metalmatze deleted the no-ksonnet-thanos-bucket branch October 21, 2020 16:25
@yeya24
Copy link
Contributor

yeya24 commented Oct 21, 2020

I am curious about the motivation behind this. So we don't want to use the ksonnet library anymore?

@metalmatze
Copy link
Contributor Author

In the end this is about readability mostly.
It's 10x easier for people to read and write this kind of code, whereas ksonnet only adds so much type safety.
Over the last couple of years it became more and more obvious that we should focus on the first one and use some other tool to check the output. kubeval so far has been really helpful.

@yeya24
Copy link
Contributor

yeya24 commented Oct 21, 2020

That makes sense. Thank you!

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

Successfully merging this pull request may close these issues.

3 participants