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

tempo-mixin: add Makefile and CI check #957

Merged
merged 12 commits into from
Sep 14, 2021

Conversation

kvrhdn
Copy link
Member

@kvrhdn kvrhdn commented Sep 14, 2021

What this PR does:

  • Adds a Makefile to simplify generating the tempo-mixin yamls.
  • Link this Makefile to the root-level Makefile so you can do make tempo-mixin.
  • Verify tempo-mixin has been updated in CI.

Should avoid issues like #956

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@kvrhdn
Copy link
Member Author

kvrhdn commented Sep 14, 2021

☝🏻 CI fails on Check tempo-mixin because #956 hasn't been merged yet.

https://github.com/grafana/tempo/runs/3599938769

@kvrhdn
Copy link
Member Author

kvrhdn commented Sep 14, 2021

Hmm, it's still failing 😞

@kvrhdn
Copy link
Member Author

kvrhdn commented Sep 14, 2021

Okay, the diff is caused by trailing newlines added by the C++ Jsonnet but not by go-jsonnet: google/go-jsonnet#518
These newlines are only added when using -m

@kvrhdn
Copy link
Member Author

kvrhdn commented Sep 14, 2021

Works now 😌

@kvrhdn kvrhdn marked this pull request as ready for review September 14, 2021 15:11
Copy link
Member

@joe-elliott joe-elliott left a comment

Choose a reason for hiding this comment

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

nice!

@joe-elliott joe-elliott merged commit 78d8107 into grafana:main Sep 14, 2021
@kvrhdn kvrhdn deleted the tempo-mixin-makefile branch September 14, 2021 19:33
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.

2 participants