-
Notifications
You must be signed in to change notification settings - Fork 6
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
Bump thrift from 0.14.1 to 0.19.0 #3
Conversation
30dad63
to
8365c03
Compare
cc @yurishkuro |
8365c03
to
8f8dc47
Compare
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
8f8dc47
to
5d703b6
Compare
uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # v3.0.0 | ||
with: | ||
username: ${{ secrets.DOCKER_USERNAME }} | ||
password: ${{ secrets.DOCKER_PASSWORD }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These were missing so the workflow failed. I just added them. Can you add on: workflow_dispatch:
to the workflow triggers?
@mmorel-35 just curious, do you actually need this? Are you still using Thrift-based Jaeger model? |
I don’t use it myself but as it is used in https://github.com/jaegertracing/jaeger/blob/main/Makefile.Thrift.mk I thought it would be appropriate to keep it up to date |
More importantly is that it adds ARM64, I can actually run it on my laptop :-) The build is running now, let's see if it succeeds and I will do a release. |
0.19 image published, trying it out in Jaeger jaegertracing/jaeger#5080 Caching works great - the tagged release workflow ran in less than a minute (has to be on the same branch though). |
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test