multiformats / multiaddr

Composable and future-proof network addresses
https://multiformats.io/multiaddr
MIT License
419 stars 84 forks source link

Check if multicodec table is in sync with the multiaddr table #142

Closed MarcoPolo closed 1 year ago

MarcoPolo commented 1 year ago

Summary

Adds a simple check to make sure codecs defined in the multiaddr table are in sync with the multicodec table. See the github workflow run for this PR as an example.

Before Merge

MarcoPolo commented 1 year ago

https://github.com/multiformats/multicodec/pull/294 should fix the failing workflow.

MarcoPolo commented 1 year ago

This test seems useful. I’m a bit hesitant since I’m not familiar with Clojure, and this code could become hard to maintain.

Would you prefer bash instead ;) ?

Joking aside, I considered doing this in many other languages, but found the Clojure version more concise and simpler (and more fun!). The code is 60 lines, and there's no esoteric features here. However, if you want to write the Go version, feel free!

marten-seemann commented 1 year ago

Would you prefer bash instead ;) ?

This could be a sharness test! 🤪

MarcoPolo commented 1 year ago

CI now passes after https://github.com/multiformats/multicodec/pull/294 was merged

BigLep commented 1 year ago

This test seems useful. I’m a bit hesitant since I’m not familiar with Clojure, and this code could become hard to maintain.

I think this is an important point. I'm worried about us introducing new languages without it being deliberate. JS or Python seem more likely for new folks to understand than Clojure.

MarcoPolo commented 1 year ago

I think this is an important point. I'm worried about us introducing new languages without it being deliberate. JS or Python seem more likely for new folks to understand than Clojure.

This is a fair point, and I should have done a better job explaining the design decision. Here's how I chose Clojure:

So Clojure was chosen because of the combination of it being the right tool for the job and long term maintenance being negligible.

BigLep commented 1 year ago

Thanks for sharing @MarcoPolo - the reasoning makes sense.

Completely agreed about not using Bash.

I'm less-worried about this specific instance given this is not an area where we're we expect to be adding a lot more scripting.

My biggest worry is about the precedent as I want to make sure that introducing new languages and technologies is very deliberate and thought through from a long-term view.

I don't think we need to discuss more now, as long as we have the takeaway for an upfront conversation for the next time, especially if it is in an area we expect to build on or expand.

Thanks.