-
Notifications
You must be signed in to change notification settings - Fork 290
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
ipld: consider moving into separate repo #296
Comments
@liamsi, curious what info channel I missed where @evan-forbes reported that. |
ref: celestiaorg/celestia-node/issues/1 |
Since we have to keep the DataAvailabilityHeader in celestia-core, and will already have to import that in celestia-node, I actually think we should keep logic needed by both repos in an independent package(s) in celestia-core. Just to have one less repo to keep track of versions. |
Having repo segregation is much cleaner in terms of code and the issue you create for that code. In long term, IMO, this only gives profits. The two problems with that are, as you've mentioned, versioning and CI. For the first on it is not a problem for me and I can handle that, for the second, we would need a unified CI, which we can postpone. |
Per a sync discussion with @Wondertan I am now convinced that we actually should move the plugin, DAH, and common logic to a different repo, provided that we have a strict release policy. I think that's what was meant by
This policy needs to ensure that the same versions of each repo are being used across the board per each release. It should also be noted that we include this policy in our docs. |
Another thing is that we should always track that releases of top-level repos rely on the number releases of repo deeper in the tree, not on the specific commits. However, that's ok if we depend on specific commits in branches. |
can we close this? |
I think it is OK to close this. I think opening a super short issue in node which points to the deleted code could help to find the relevant portions. |
IPLD code is linked in celestia-node issue #106, but just in case we need a convenient lookup later : |
Summary
Optimint and other projects might use the current code under ipld:https://github.com/lazyledger/lazyledger-core/tree/40acb17283ebfe1c49b804a786464b93c2213a97/p2p/ipld
It makes sense to eventually move this out into a separate repository once the APIs stabilized a bit.
Action Items
Context / Original Openening Comment
Decide if we want to support using the IPLD plugin easily with vanilla IPFS. Previously, this was extensively used for testing and there is even a unit test, that required this logic (currently skipped):
https://github.com/lazyledger/lazyledger-core/blob/874df76f139b4a3effcd2a74d6078c9d08b6d1ba/p2p/ipld/plugin/nodes/nodes_test.go#L129-L130
Merging #294 will make this less easy to actually deploy an ipfs node with the plugin because building it with the Makefile will use some of the dependencies from the lazyledger-core go.sum. These dependencies most likely will differ from the deps used in the IPFS version (even if compiled locally). e.g. @evan-forbes noticed that after compiling the version in #294
plugin.Open("/redacted/.ipfs/plugins/lazyledger-plugin"): plugin was built with a different version of package golang.org/x/sys/cpu
.There certainly might be a way to get this working without moving the plugin into a separate repository but I think it would be easier and it also would make sense.
If we decide to go down that route, we should definitely keep the git history intact (which is possible).
The text was updated successfully, but these errors were encountered: