refactor!: Move ConnectionId and PendingPoint to libp2p-swarm

[umgefahren]

Description

Both of these are only needed as part of libp2p-swarm. Them residing in libp2p-core is a left-over from when libp2p-core still contained Pool.

Closes #3200.

Notes

As requested by @thomaseizinger I moved ConnectionId from core to swarm. I also changed every use of the struct from core to swarm.

I moved the ConnectionId struct and changed every relevant use. I also want to talk about the necessity of operator overloading.

Links to any relevant issues

Open Questions

None

Change checklist

• I have performed a self-review of my own code
• A changelog entry has been made in the appropriate crates

[jxs]

Looks good to me, it's just missing the CHANGELOG.md entries and the Cargo.toml bumps

[thomaseizinger]

Thanks @umgefahren !

This is a breaking change albeit a trivial one. I think we should hold off merging this until we merge something breaking that we definitely want.

For one, this is going to save trivial PRs like this one from making a lot of version bumps and changelog entries. Additionally, it allows us to accumulate features in master without breaking changes. Which is a good thing IMO.

Thoughts @libp2p/rust-libp2p-maintainers ?

[elenaf9]

This is a breaking change albeit a trivial one. I think we should hold off merging this until we merge something breaking that we definitely want.

For one, this is going to save trivial PRs like this one from making a lot of version bumps and changelog entries. Additionally, it allows us to accumulate features in master without breaking changes. Which is a good thing IMO.

Thoughts @libp2p/rust-libp2p-maintainers ?

Don't have a strong opinion but sounds reasonable.
What do you think about adding a label for such low-priority-with-breaking-changes PRs, so that once we break master anyway we know what PRs can be merged?

[thomaseizinger]

This is a breaking change albeit a trivial one. I think we should hold off merging this until we merge something breaking that we definitely want.
For one, this is going to save trivial PRs like this one from making a lot of version bumps and changelog entries. Additionally, it allows us to accumulate features in master without breaking changes. Which is a good thing IMO.
Thoughts @libp2p/rust-libp2p-maintainers ?

Don't have a strong opinion but sounds reasonable. What do you think about adding a label for such low-priority-with-breaking-changes PRs, so that once we break master anyway we know what PRs can be merged?

Given that we know what the next version major will be, we could also create a milestone to plan it. That could also come in handy for deprecations that we've been making: We often open a PR already at the time of deprecating things where we remove the deprecated items. Those could also be added to such a milestone.

[mergify]

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

[umgefahren]

Looks good to me, it's just missing the CHANGELOG.md entries and the Cargo.toml bumps

Done! Why is the CI still failing? I thought that was fixed. We should really transition away from protoc...

[mergify]

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

[mxinden]

In case we merge #3196 and thus release a breaking change of libp2p-core, we can include this pull request as well.

[thomaseizinger]

Looks good to me, it's just missing the CHANGELOG.md entries and the Cargo.toml bumps

Done! Why is the CI still failing? I thought that was fixed. We should really transition away from protoc...

It is being worked on :)

[thomaseizinger]

Please also move PendingPoint as part of this and make it pub(crate).

[mxinden]

#3196 is merged, thus we can easily include it in the next release cycle if you like @umgefahren.

[umgefahren]

#3196 is merged, thus we can easily include it in the next release cycle if you like @umgefahren.

Sounds good to me. I have to resolve the merge conflicts first

[thomaseizinger]

Friendly ping @umgefahren. I want this for #3254, let me know if you are still working on this, otherwise I'll push it forward!

[umgefahren]

Please go ahead. I’m still preparing for exams

[thomaseizinger]

Targeting #3318 to have CI green. Moved to draft until #3318 is merged.

[thomaseizinger]

Ah damn mergify, can't stack PRs from forks ...