libp2p / go-libp2p-pubsub

The PubSub implementation for go-libp2p
https://github.com/libp2p/specs/tree/master/pubsub
Other
309 stars 178 forks source link

Update to go-libp2p v0.25 #517

Closed MarcoPolo closed 1 year ago

MarcoPolo commented 1 year ago

I'm curious to see the test results in CI, because for me the tests are very flaky.

MarcoPolo commented 1 year ago

Wow tests pass in CI but not my machine.

Jorropo commented 1 year ago

I just spent fives minutes doing this, :facepalm: :pray:

masih commented 1 year ago

@Jorropo May I offer rum in exchange for releasing this commit please? 🥃

Jorropo commented 1 year ago

@masih is that a good or bad point ? :rofl: This repo does not have uCI, I got to do it by hand. :upside_down_face: (git pull && git tag v0.9.0 && git push origin v0.9.0)

masih commented 1 year ago

The promise of rum is secured regardless; thank you for the speedy release 🙏

vyzo commented 1 year ago

Really, what was the point of this release?

Pubsub only uses core interfaces really (modulo tests) and there should be absolutely no reason to upgrade libp2p unless there are breaking changes.

Also note that you are forcing downstream users to upgrade libp2p and its new bugs, which is really undesirable. Pubsub users should be free to use whatever minimum version they have vetted and tested with.

Please avoid doing this again in the future, unless there is a really good reason.

masih commented 1 year ago

Code wouldn't compile without it I'm afraid:

github.com/libp2p/go-libp2p-pubsub@v0.8.1/pubsub.go:314:32:  cannot use ps.protoMatchFunc(string(id)) (value of type func(string) bool) as type func(protocol.ID) bool in argument to h.SetStreamHandlerMatch
vyzo commented 1 year ago

Ok, so there was a breaking change.

Fair enough, but please list those reasons in the pr in the future, so that other users are aware of the incompatibility.

masih commented 1 year ago

Of course! thanks @vyzo