-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
When to release 1.22 #2734
Comments
I was kinda expecting to release a version bump fixing the Thrift issue for the agent. Do you think it would be worth forking Thrift for the time being, so that we can tag our fork to be able to use the fix? |
Also, we have a couple of things in the milestone: https://github.com/jaegertracing/jaeger/milestone/16 |
perhaps. Not sure how much work it is. |
I might be wrong, but I think it's just a matter of forking/tagging for the Go library. I can try to schedule some time to experiment with this. |
Added #2645 |
I think we are ready for the release. Following the tradition, the newly appointed maintainer is invited to perform the release ;-) cc @albertteoh |
I would be honoured, working on it now |
I'm planning to tag a jaeger-ui release, but master builds are failing due to a license scan check. I'm looking into the license scan error, but unable to proceed identifying the problematic file as I'm not a maintainer it seems: Is someone able to help with this or can the license check be ignored and proceed with the release tag? |
Link? I can check access. |
Thanks @yurishkuro. Here's the link. For context, I got there by going to https://github.com/jaegertracing/jaeger-ui, and clicking on the red "X" on the last commit, then clicking on the license check failed build. |
I don't have access either! |
I sent email to FOSSA support. I don't think this is a new issue though, we can probably ignore it. Most likely the warning is coming from some build-time dependencies. |
btw, would prefer to do the UI release without merging all those dependabot upgrades, I think they need a round of sanity check testing of the UI (unfortunately, unit tests do not fully cover UI). |
okay, noted; please let me know if you'd like me to revert those last two merges I did (a few hours ago). Otherwise, I'll avoid merging the dependabot upgrades going forward. |
Let's hold it until next Tuesday morning (your timezone, @albertteoh). I can do some manual testing on Monday. |
Sounds good, @jpkrohling, I'll wait till Tuesday. I did perform some (basic) manual testing and things seemed fine so far, though I'm sure there are a number of features that I'm not familiar with. On a related note, if this is something we'll need to do each time before tagging a new UI release, do you think we ought to have a document outlining manual sanity testing steps until we have enough confidence in our unit tests? Or maybe even invest in an automated UI testing framework like selenium? |
Absolutely, I was thinking about recording a BDD-style set of tests as I go through it this morning, but I might not have enough time. Feel free to suggest an initial set of tests, I can certainly help to review it! |
I just tried the latest master branches (d9453e0 and jaegertracing/jaeger-ui@35cc879) and it looks fine to me. One thing that I would like to get in before the release is the jaegertracing/jaeger-ui#702, as it fixes a CVE that was reported to me last week. |
Thanks @jpkrohling, I noticed jaegertracing/jaeger-ui#702 is merged now so I'll go head with tagging jaeger-ui, then create the PR for jaeger release v1.22.0. |
Cool, I've created an issue for this: jaegertracing/jaeger-ui#708. |
Remove temporary usage of a Thrift fork #2781 Remove temporary usage of a personal Thrift forkThe text was updated successfully, but these errors were encountered: