Skip to content
This repository has been archived by the owner on Sep 6, 2022. It is now read-only.

Investigate using tooling to prevent compatibility breakages #39

Closed
Kubuxu opened this issue Jul 15, 2019 · 8 comments · Fixed by #40
Closed

Investigate using tooling to prevent compatibility breakages #39

Kubuxu opened this issue Jul 15, 2019 · 8 comments · Fixed by #40

Comments

@Kubuxu
Copy link
Member

Kubuxu commented Jul 15, 2019

There exist tooling that will detect compatibility breaking changes. It would alert us in cases like recently when a new version of insecure transport or addition of event bus broke the compatibility.
One of the tools is https://github.com/smola/gocompat
The idea would be to run it in CI for every PR and compare the compatibility with the latest release/master.

Regarding compatibility: https://blog.merovius.de/2015/07/29/backwards-compatibility-in-go.html is worth a read.

Would help in cases like #23 #37 #38 libp2p/go-libp2p#678

@raulk
Copy link
Member

raulk commented Jul 15, 2019

I think it's simpler than that. Just having a CI build on https://github.com/libp2p/workspace-go-libp2p that:

  1. refreshes all HEADs.
  2. runs ./workspace.sh local to add the replace directives.
  3. runs a full build (git submodule foreach "go build ./...") should be enough.

@Kubuxu
Copy link
Member Author

Kubuxu commented Jul 15, 2019

That wouldn't prevent breakages on interfaces that are only used by external applications.

@raulk
Copy link
Member

raulk commented Jul 15, 2019

That wouldn't prevent breakages on interfaces that are only used by external applications.

Don't we exercise those in tests? Wouldn't those break there?

@Kubuxu
Copy link
Member Author

Kubuxu commented Jul 15, 2019

Another example:
Let's say we decide to add some functions to an interface; let's say routing. All of our implementations implement that functions, so it is not a problem, but it is still a breaking change. If all of our implementations were implementing that function, we would not see failing tests.

@raulk
Copy link
Member

raulk commented Jul 15, 2019

@Kubuxu Yeah, indeed we do have two surfaces for breakage.

  1. I do believe we need mechanisms to detect API breakage first internally, then we can bother to produce reporting for external breakage.
  2. Following the advice in https://blog.merovius.de/2015/07/29/backwards-compatibility-in-go.html == ossification from the get-go. The article does make a valid point, but software is in continuous evolution (as the author recognises). Some counterarguments:
    • Developers embedding types and interfaces directly know what the risks of that practice are.
    • The practice of finding your users via godoc.org and continuously integrating upstream changes is interesting, but does not scale, and it encourages a spirit of "it's ok to break things, because we can warn the people who depend on us".
    • We need to apply CI internally first, as I've suggested.
    • libp2p is at version v0; it will continue to evolve heavily.

I think what would've helped in #23 #37 #38 libp2p/go-libp2p#678 is continuous master integration in workspace-go-libp2p. gocompat is a nice addition, but we have a more fundamental flaw in our software engineering here.

I'd also like to point out much of the grief we're experiencing is due to the multirepo approach to modularity via code segregation. 💣

@Kubuxu
Copy link
Member Author

Kubuxu commented Jul 15, 2019

My point is to have the tooling to detect and know that there is a breaking change. I'm not proposing policy about these changes.

I've created #40 the PR contains an example of breaking change in a separate commit, to showcase what tooling gives us.


This is entirely off topic:

I'd also like to point out much of the grief we're experiencing is due to the multirepo approach to modularity via code segregation.

I disagree with it very strongly. It shows us, there is a considerable cost of having an unstable API.

@Kubuxu
Copy link
Member Author

Kubuxu commented Jul 15, 2019

I'm also not dismissing a need for org-wide CI, but in my opinion, that effort is orthogonal to this one.

@Kubuxu
Copy link
Member Author

Kubuxu commented Jul 15, 2019

In my opinion, the reason for these breaking changes was because people creating them and people accepting them weren't reviewing them for that scope.
In my opinion, if we can replace a part of the review process with tooling we should. Especially if we have an experience that human review process fails in this given case.

@raulk raulk closed this as completed in #40 Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants