-
Notifications
You must be signed in to change notification settings - Fork 100
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
Clarify ipld/car relatioship with ipld/go-car #244
Conversation
Codecov Report
@@ Coverage Diff @@
## main #244 +/- ##
==========================================
- Coverage 48.23% 48.13% -0.11%
==========================================
Files 270 270
Lines 33064 33064
==========================================
- Hits 15950 15915 -35
- Misses 15455 15482 +27
- Partials 1659 1667 +8 |
The dependency issue here seems to be on the There are only a couple calls / references in there, and the definition of what a block is will be really quite stable at this point. go-car could remove this dependency with a bit of refactoring by defining the interface locally, so that it can be compatible with either of these implementations. If there wasn't a dependency re-factor on |
I believe tha's right.
Cool!
Per #218 (comment) , go car libraries isn't where we were wanting to apply maintenance or innovation energy. I know the team is comfortable building and working with car tooling if necessary (e.g., @Jorropo's different car related libraries in https://github.com/Jorropo/linux2ipfs and https://github.com/Jorropo/go-featheripfs). I expect the team will want a break for a month or two before considering getting rid of boxo/ipld/car given the fatigue on all the renaming and path updating the last month. I know I'm being non-committal here, not because have another secret plan, but because don't know at this point.
Good point. I don't think so: https://github.com/ipld/go-car/issues?q=is%3Aissue+jorropo+ |
The angle I'm taking here is that there have been a number of PRs over the last year to fix bugs or otherwise improve the car-v1 codebase that is in the go-car repo (example 1 2 3). By having a separate instance in boxo, there's a duplicate maintenance burden we're incurring. In the meantime we probably want to have some sense of the expected process: - is it on go-car to notify boxo that there's a PR there that probably should get pulled into the boxo implementation and vice versa? or should the boxo maintainers be watching go-car for such relevant PRs and vice versa? |
Adjust migration tool config so that don't update users to boxo/ipld/car by default.
c6a0da2
to
3c9aaec
Compare
I looked into potentially providing an interface, but this seems to not be the case. The boxo fork of car is using the same external As such, I don't think there's a refactor-needed / incompatibility in linking against the external ipld/go-car as it is. If that isn't true we should track down what it is we are proposing needs to be done. |
Good point @willscott - thanks for raising. I think Boxo in general needs a mechanism here to know if all the copied repos (including ipld/go-car) have received "important updates". I created #270 to track this work (and included an for how dependabot could be leveraged). For ipld/go-car specifically, I don't want you and the current maintainers to have to do anything extra than you already do for notifying consumers of releases/changes. I assume that happens through the normal github release and dependabot flow, but if there is anywhere else we should be watching let us know. Certainly if there are important bug fixes or performance improvements, we're not going to say "no" to extra communication in #boxo-maintainers FIL slack or ipfs/boxo issue, but that is not expected. Thanks! |
That makes sense - my angle here has been to make sure that if nothing else, the pull from the ipld instance can remain as trivial as possible - so making sure we understand what would prevent the code from being identical between the two. |
soon → in the future since there aren't specific plans here (if there were, there should be an issue describing it)
@willscott : concerning |
Remaining work here @Jorropo :
|
Per IPFS Thing 2023 conversations, we are going to be able to remove Boxo's ipld/go-car copy from Boxo. When that happens, this PR can just be closed. |
I'm closing this PR since we'll be removing boxo/car per #218 (comment) |
See conversation in #218 (comment)