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

Update several dependencies #1855

Closed
wants to merge 6 commits into from
Closed

Conversation

zalegrala
Copy link
Contributor

@zalegrala zalegrala commented Nov 1, 2022

Signed-off-by: Zach Leslie zach.leslie@grafana.com

What this PR does:

This PR updates several depenencies as part of an update for weaveworks/common to allow TLS specifications.

Checklist

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

github.com/stretchr/testify v1.8.0
github.com/thanos-io/thanos v0.24.0
github.com/stretchr/testify v1.8.1
github.com/thanos-io/thanos v0.29.0-rc.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a big one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do we feel about an rc here? We could wait, or take the rc and quickly update when 29.0 comes.

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the motivation for upgrading to 0.29.0-rc.0 vs the latest stable which I assume is 0.28.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a recent breaking change in one of the dependencies that a bunch of projects share.

https://github.com/thanos-io/thanos/blob/v0.28.1/go.mod#L19

This caused a bunch of headache for updates, which has been updated in 29.0-rc.0 to point to the new location.

Signed-off-by: Zach Leslie <zach.leslie@grafana.com>
Signed-off-by: Zach Leslie <zach.leslie@grafana.com>
Signed-off-by: Zach Leslie <zach.leslie@grafana.com>
Signed-off-by: Zach Leslie <zach.leslie@grafana.com>
Signed-off-by: Zach Leslie <zach.leslie@grafana.com>
@zalegrala zalegrala marked this pull request as ready for review November 2, 2022 01:19
Signed-off-by: Zach Leslie <zach.leslie@grafana.com>
@zalegrala
Copy link
Contributor Author

Let's hold on this. I might be having local issues that prevented me from making a smaller update.

@bboreham
Copy link
Contributor

bboreham commented Nov 3, 2022

Note Tempo already has the TLS settings; the current depenency includes weaveworks/common#256.
The only diff between that and latest weaveworks/common is in the wording of the help text.

@zalegrala
Copy link
Contributor Author

Let's close this one out. I've raised #1857 as a replacement update.

@zalegrala zalegrala closed this Nov 3, 2022
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