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

[2023-04-25] Bump dependencies identified by dependabot #15776

Merged
merged 1 commit into from
Apr 25, 2023

Conversation

@fuweid fuweid marked this pull request as ready for review April 25, 2023 07:15
@fuweid fuweid changed the title Bump dependencies identified by dependabot [2023-04-25] Bump dependencies identified by dependabot Apr 25, 2023
@ahrtr
Copy link
Member

ahrtr commented Apr 25, 2023

Based on Indirect dependencies, usually we don't bump a dependency if all modules just indirectly depend on it, such as github.com/benbjohnson/clock.

Note that uber-go/zap/blob/v1.24.0 depends on github.com/benbjohnson/clock 1.1.0, but it already bumped the dependency to 1.3.0 in its main branch. Next time when we bump go.uber.org/zap, the github.com/benbjohnson/clock will be automatially updated.

@fuweid
Copy link
Member Author

fuweid commented Apr 25, 2023

Hi @ahrtr

For github.com/benbjohnson/clock dep, I will delete the commit in this pr.

I have a question about indirect deps in tools/cmd,

The github.com/hexfusion/schwag requires the following two deps. But schwag hasn't been updated for a long time.
For this case, we just ignore it, right?

  • github.com/go-openapi/spec
  • github.com/go-openapi/jsonpointer

And for the publicsuffix-go and sqlx deps, they are required by cfssl command. So we should handle it next time when cfssl is updated. Is it correct?

If my understand is correct, this pull-request only needs to bump one dep nakedret.

@ahrtr
Copy link
Member

ahrtr commented Apr 25, 2023

If my understand is correct, this pull-request only needs to bump one dep nakedret.

Correct.

The github.com/hexfusion/schwag requires the following two deps.

I suggest to remove the dependency on github.com/hexfusion/schwag; of course, in a separate PR.

So we should handle it next time when cfssl is updated. Is it correct?

github.com/cloudflare/cfssl--> github.com/zmap/zcrypto --> github.com/weppos/publicsuffix-go.

We should push github.com/cloudflare/cfssl to bump github.com/zmap/zcrypto firstly.

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @fuweid

next turn: @chaochn47

@ahrtr ahrtr merged commit 9b310ea into etcd-io:main Apr 25, 2023
@ahrtr
Copy link
Member

ahrtr commented Apr 25, 2023

@fuweid could you please close all the PRs opened by dependabot?

@fuweid fuweid deleted the update-deps branch April 25, 2023 08:42
@fuweid
Copy link
Member Author

fuweid commented Apr 25, 2023

@ahrtr thanks for the help 😂 I will close it by myself next time.

@ahrtr
Copy link
Member

ahrtr commented Apr 25, 2023

@fuweid could you please close all the PRs opened by dependabot?

Just closed the PRs myself.

@ahrtr
Copy link
Member

ahrtr commented Apr 25, 2023

followups:

  1. push github.com/cloudflare/cfssl to bump github.com/zmap/zcrypto, or just raise a PR for github.com/cloudflare/cfssl directly. Probably we can suggest cloudflare/cfssl to enable & configure dependabot as well.
  2. investigate how to remove the dependency on github.com/hexfusion/schwag

@fuweid
Copy link
Member Author

fuweid commented Apr 25, 2023

handling the github.com/hexfusion/schwag case right now.

@chaochn47
Copy link
Member

chaochn47 commented May 1, 2023

LGTM

Thanks @fuweid

next turn: @chaochn47

I did not see any direct dependencies needs to be bumped up this week, so I closed all of PRs opened by dependabot.

Could you please take a look if I did the right thing? @ahrtr, thanks!

@ahrtr
Copy link
Member

ahrtr commented May 1, 2023

Thanks @chaochn47. It's correct to close the PRs bumping indirect dependencies.

I see that the PR #15806 isn't closed. It isn't an indirect dependency, but I vaguely remember that it's also causing incompatibility issue, and it couldn't pass the workflow checks. I am not sure whether you have already double checked it. Are you raising separate PR for it?

@chaochn47
Copy link
Member

Good catch, somehow I missed that PR #15806, will raise a separate PR for it otel version bump up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants