Skip to content
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

chore: Remove ipfslite dependency #739

Merged
merged 6 commits into from
Oct 18, 2022

Conversation

fredcarle
Copy link
Collaborator

@fredcarle fredcarle commented Aug 12, 2022

Relevant issue(s)

Resolves #738

Description

This PR removes our direct dependency on ipfslite. It is still an indirect dependency simply because it's used by an other package we import. Not from a function that we use but from a test of that package.

Note that some commented code will need to be uncommented once #683 is resolved. Although a ds.Batching implementing datastore needs to be used for the DHT within the net package, as long as badgerds.Datastore keeps the Batch method, the should have no problem.

Tasks

  • I made sure the code is well commented, particularly hard-to-understand areas.
  • I made sure the repository-held documentation is changed accordingly.
  • I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
  • I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

unit tests

Specify the platform(s) on which this was tested:

  • MacOS

@fredcarle fredcarle added area/p2p Related to the p2p networking system action/no-benchmark Skips the action that runs the benchmark. dependencies Related to dependencies labels Aug 12, 2022
@fredcarle fredcarle added this to the DefraDB v0.3.1 milestone Aug 12, 2022
@fredcarle fredcarle self-assigned this Aug 12, 2022
@fredcarle fredcarle marked this pull request as draft August 12, 2022 17:05
@fredcarle fredcarle force-pushed the fredcarle/chore/I738-remove-ipfslite-dependency branch 2 times, most recently from 07df391 to 088ae96 Compare August 12, 2022 18:56
@codecov
Copy link

codecov bot commented Aug 12, 2022

Codecov Report

Merging #739 (6d851bb) into develop (91cd7a3) will increase coverage by 0.05%.
The diff coverage is 83.11%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #739      +/-   ##
===========================================
+ Coverage    59.78%   59.84%   +0.05%     
===========================================
  Files          156      156              
  Lines        17358    17415      +57     
===========================================
+ Hits         10378    10422      +44     
- Misses        6044     6056      +12     
- Partials       936      937       +1     
Impacted Files Coverage Δ
net/peer.go 32.19% <55.00%> (+1.27%) ⬆️
node/node.go 66.10% <92.85%> (+6.00%) ⬆️
net/server.go 55.93% <100.00%> (-1.54%) ⬇️
connor/lt.go 72.72% <0.00%> (-4.55%) ⬇️
events/simple.go 86.76% <0.00%> (+2.94%) ⬆️

@orpheuslummis
Copy link
Contributor

One thing in favor of this PR is that is allows to stop maintaining the custom fork https://github.com/sourcenetwork/ipfs-lite

node/node.go Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
@fredcarle fredcarle modified the milestones: DefraDB v0.3.1, DefraDB v0.4 Sep 16, 2022
@fredcarle fredcarle force-pushed the fredcarle/chore/I738-remove-ipfslite-dependency branch from 6c29ae8 to b5f3675 Compare October 16, 2022 05:09
@fredcarle fredcarle changed the title WIP: remove ipfslite dependency chore: Remove ipfslite dependency Oct 16, 2022
@fredcarle fredcarle marked this pull request as ready for review October 16, 2022 05:21
@fredcarle fredcarle requested a review from a team October 16, 2022 05:23
This was referenced Oct 17, 2022
go.mod Show resolved Hide resolved
@orpheuslummis
Copy link
Contributor

suggestion: when this merges, also archive the https://github.com/sourcenetwork/ipfs-lite repository

Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me :) Couple of minor non-blocking suggestions and am looking forward to this going in

net/peer.go Outdated Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
node/node.go Show resolved Hide resolved
node/node.go Outdated Show resolved Hide resolved
@fredcarle fredcarle force-pushed the fredcarle/chore/I738-remove-ipfslite-dependency branch from 48f1745 to d30fa2d Compare October 18, 2022 20:52
@fredcarle fredcarle merged commit a82ae40 into develop Oct 18, 2022
@fredcarle fredcarle deleted the fredcarle/chore/I738-remove-ipfslite-dependency branch October 18, 2022 22:48
shahzadlone pushed a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
Relevant issue(s)
Resolves sourcenetwork#738

Description
This PR removes our direct dependency on ipfslite. It is still an indirect dependency simply because it's used by an other package we import. Not from a function that we use but from a test of that package.

Note that some commented code will need to be uncommented once sourcenetwork#683 is resolved. Although a ds.Batching implementing datastore needs to be used for the DHT within the net package, as long as badgerds.Datastore keeps the Batch method, the should have no problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/p2p Related to the p2p networking system dependencies Related to dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove ipfslite dependency
3 participants