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

Bump thrift from 0.14.1 to 0.19.0 #3

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

mmorel-35
Copy link

@mmorel-35 mmorel-35 commented Dec 27, 2023

Which problem is this PR solving?

  • thrift 0.19.0 is available

Description of the changes

  • update thrift from 0.14.1 to 0.19.0
  • create a github workflow to lint and publish jaegertracing/thrift
  • remove the previous Dockerfiles as they won’t be needed anymore
  • QEMU steps are copied from docker-protobuf

How was this change tested?

  • CI

Checklist

@mmorel-35 mmorel-35 force-pushed the thrift-0.19.0 branch 4 times, most recently from 30dad63 to 8365c03 Compare December 28, 2023 06:32
@mmorel-35
Copy link
Author

cc @yurishkuro

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
@yurishkuro yurishkuro merged commit 0757fcc into jaegertracing:master Jan 3, 2024
2 checks passed
uses: docker/login-action@343f7c4344506bcbf9b4de18042ae17996df046d # v3.0.0
with:
username: ${{ secrets.DOCKER_USERNAME }}
password: ${{ secrets.DOCKER_PASSWORD }}
Copy link
Member

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 mmorel-35 deleted the thrift-0.19.0 branch January 3, 2024 16:00
@yurishkuro
Copy link
Member

@mmorel-35 just curious, do you actually need this? Are you still using Thrift-based Jaeger model?

@mmorel-35
Copy link
Author

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

@yurishkuro
Copy link
Member

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.

@yurishkuro
Copy link
Member

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).

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