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

deps!: Update multiaddr & multihash to 0.17.0 #3196

Merged
merged 8 commits into from
Dec 20, 2022

Conversation

ppodolsky
Copy link
Contributor

@ppodolsky ppodolsky commented Dec 6, 2022

Description

Notes

Dependabot forgot to update multihash in https://github.com/libp2p/rust-libp2p/pull/3193/files, fixed this.

Depends-On: #3203

@ppodolsky ppodolsky force-pushed the dependabot/cargo/multiaddr-0.17.0 branch from bff3fd3 to 61a1b53 Compare December 6, 2022 08:04
@ppodolsky ppodolsky changed the title Update multiaddr to 0.17.0 build(deps): Update multiaddr to 0.17.0 Dec 6, 2022
@b5
Copy link

b5 commented Dec 6, 2022

👋 libp2p team, would love to know if this is mergable. It's a blocker on n0-computer/iroh#546 for us in iroh. Thanks!

@thomaseizinger thomaseizinger changed the title build(deps): Update multiaddr to 0.17.0 deps: Update multiaddr to 0.17.0 Dec 7, 2022
@thomaseizinger thomaseizinger changed the title deps: Update multiaddr to 0.17.0 deps!: Update multiaddr to 0.17.0 Dec 7, 2022
@thomaseizinger thomaseizinger changed the title deps!: Update multiaddr to 0.17.0 deps!: Update multiaddr & multihash to 0.17.0 Dec 7, 2022
Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

I kicked CI.

@thomaseizinger
Copy link
Contributor

The failing interop test should be fixed once #3203 is merged.
The two failing semver tests are due to a bug in cargo semver-checks: #2647 (comment)

@mxinden Will need to exercise his admin rights to merge this until we have that sorted.

@ppodolsky
Copy link
Contributor Author

@thomaseizinger Hi! Could you push it forward? Is there anything I can do to help?

@thomaseizinger
Copy link
Contributor

@thomaseizinger Hi! Could you push it forward? Is there anything I can do to help?

Nope, I don't have the admin rights to merge a PR with failing builds. We need to merge and release #3213 to fix the false-positive reports from cargo semver-checks.

@thomaseizinger
Copy link
Contributor

I've approved it to go into the merge-queue but that won't happen until the build is green. Keep an eye on the conversation in #3213. Once that is resolved and released, you should be able to get this in by updating it with latest master to trigger CI again at which point it should automatically go in once it is green!

Copy link
Member

@mxinden mxinden left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @ppodolsky.

core/Cargo.toml Show resolved Hide resolved
@mxinden mxinden removed the send-it label Dec 12, 2022
@mergify
Copy link
Contributor

mergify bot commented Dec 12, 2022

This pull request has merge conflicts. Could you please resolve them @ppodolsky? 🙏

@mxinden
Copy link
Member

mxinden commented Dec 14, 2022

@ppodolsky I seem to not have permissions to push to your branch. Can you please cherry pick mxinden@72248a1 ?

@thomaseizinger any objections including this in the next release? In other words, any objections in doing a full breaking change for the next release?

@ppodolsky
Copy link
Contributor Author

Done

@thomaseizinger
Copy link
Contributor

@thomaseizinger any objections including this in the next release? In other words, any objections in doing a full breaking change for the next release?

Nope, fine with me!

thomaseizinger
thomaseizinger previously approved these changes Dec 19, 2022
transports/webrtc/CHANGELOG.md Outdated Show resolved Hide resolved
transports/tls/CHANGELOG.md Outdated Show resolved Hide resolved
transports/tls/CHANGELOG.md Outdated Show resolved Hide resolved
@thomaseizinger
Copy link
Contributor

Mergify can't update your branch to the latest version of master: https://github.com/libp2p/rust-libp2p/pull/3196/checks?check_run_id=10175104797

Can you see if you can address this? I've heard that it is tricky to do for forks coming from organisations but haven't confirmed that. Worst case, can you open this PR from a personal fork instead?

dependabot bot and others added 6 commits December 19, 2022 07:50
Updates the requirements on [multiaddr](https://github.com/multiformats/rust-multiaddr) to permit the latest version.
- [Release notes](https://github.com/multiformats/rust-multiaddr/releases)
- [Changelog](https://github.com/multiformats/rust-multiaddr/blob/master/CHANGELOG.md)
- [Commits](multiformats/rust-multiaddr@v0.16.0...v0.17.0)

---
updated-dependencies:
- dependency-name: multiaddr
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
@mergify mergify bot dismissed thomaseizinger’s stale review December 19, 2022 06:51

Approvals have been dismissed because the PR was updated after the send-it label was applied.

@ppodolsky
Copy link
Contributor Author

Mergify can't update your branch to the latest version of master: https://github.com/libp2p/rust-libp2p/pull/3196/checks?check_run_id=10175104797

Can you see if you can address this? I've heard that it is tricky to do for forks coming from organisations but haven't confirmed that. Worst case, can you open this PR from a personal fork instead?

Done, but had to force push rebased commits.
I believe organisations' forks are ok if the organisation belongs to you.

@thomaseizinger
Copy link
Contributor

Mergify can't update your branch to the latest version of master: #3196 (checks)
Can you see if you can address this? I've heard that it is tricky to do for forks coming from organisations but haven't confirmed that. Worst case, can you open this PR from a personal fork instead?

Done, but had to force push rebased commits. I believe organisations' forks are ok if the organisation belongs to you.

Cool thanks! Let's see if mergify is happy now :)

thomaseizinger
thomaseizinger previously approved these changes Dec 19, 2022
@mergify mergify bot dismissed thomaseizinger’s stale review December 19, 2022 08:19

Approvals have been dismissed because the PR was updated after the send-it label was applied.

@mergify
Copy link
Contributor

mergify bot commented Dec 19, 2022

This pull request has merge conflicts. Could you please resolve them @ppodolsky? 🙏

@thomaseizinger
Copy link
Contributor

Some of the dependency bumps in this PR seem to be incorrect: https://github.com/libp2p/rust-libp2p/actions/runs/3729641763/jobs/6327231245

@ppodolsky
Copy link
Contributor Author

@mxinden Seems something from your commit. Not sure what to do with it

@mxinden
Copy link
Member

mxinden commented Dec 20, 2022

Should be fixed with #3261 merged. Can you update the pull request @ppodolsky?

@mxinden
Copy link
Member

mxinden commented Dec 20, 2022

Mergify can't update your branch to the latest version of master: #3196 (checks)
Can you see if you can address this? I've heard that it is tricky to do for forks coming from organisations but haven't confirmed that. Worst case, can you open this PR from a personal fork instead?

Done, but had to force push rebased commits. I believe organisations' forks are ok if the organisation belongs to you.

Cool thanks! Let's see if mergify is happy now :)

Following up on this, for future pull requests, us being able to merge master ourselfs into your pull request would make both of our lifes a bit easier.

@mxinden
Copy link
Member

mxinden commented Dec 20, 2022

🙏

@ppodolsky
Copy link
Contributor Author

Following up on this, for future pull requests, us being able to merge master ourselfs into your pull request would make both of our lifes a bit easier.

Would be glad to allow but have no idea how to do it

@ppodolsky
Copy link
Contributor Author

Now it is clippy(beta) failing

@mxinden
Copy link
Member

mxinden commented Dec 20, 2022

Now it is clippy(beta) failing

That CI check is not required, i.e. can be fixed in a different pull request. Only the stable clippy check is required.

@mergify mergify bot merged commit 929cbb4 into libp2p:master Dec 20, 2022
@mxinden
Copy link
Member

mxinden commented Dec 20, 2022

🎉 Thanks for the help here.

mxinden added a commit to mxinden/rust-libp2p that referenced this pull request Feb 24, 2023
libp2p#3196 forgot to bump `libp2p-uds`. `libp2p-uds` was
updated to `libp2p-core` `v0.39.0` thus requiring the version bump.

This has happened before, see libp2p#2720.
@mxinden mxinden mentioned this pull request Feb 24, 2023
4 tasks
mergify bot pushed a commit that referenced this pull request Feb 24, 2023
#3196 forgot to bump `libp2p-uds`. `libp2p-uds` was updated to `libp2p-core` `v0.39.0` thus requiring the version bump.

This has happened before, see #2720.

Pull-Request: #3502.
umgefahren pushed a commit to umgefahren/rust-libp2p that referenced this pull request Mar 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants