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

[WIP] Next round of extractions (blockservice, merkledag, path) #4960

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@dignifiedquire
Member

dignifiedquire commented Apr 21, 2018

This PR extracts the following packages

  • ./blockservice -> ipfs/go-ipfs-blockservice
  • ./merkledag - > ipfs/go-ipfs-merkledag
  • ./path -> ipfs/go-ipfs-path
  • ./path/resolver -> ipfs/go-ipfs-path-resolver
  • ./thirdparty/verifcid -> ipfs/go-verifcid

I tried to extract less at once, but these are all depended on each other in some form, so ripping them out at once was the easiest way I could see

I have the individual packages ready to push, please let me know if I can go ahead and create the repos with their respective PRs.

Edit Yes gitcop, I will make you happy next time I push...

@kevina

This comment has been minimized.

Contributor

kevina commented Apr 21, 2018

Note, thirdparty/verifcid is a temporary package that will likely go away once ipfs/go-cid#40 lands so it should not be extracted.

Also, is there a strong need for this right now?

@magik6k

This comment has been minimized.

Member

magik6k commented Apr 21, 2018

#4672 and following PRs will refactor path, path/resolver and also touch merkledag. path package will likely get merged into CoreAPI, so this is going to make these refactors move slower.

cc @Stebalien

@magik6k

Looks like there are some missing changes to package.json

@dignifiedquire

This comment has been minimized.

Member

dignifiedquire commented Apr 21, 2018

@hsanjuan

This comment has been minimized.

Contributor

hsanjuan commented Apr 23, 2018

So happy to see this.

@ZenGround0 keep an eye on this, as this should allow to fully remove the go-ipfs dep from cluster/sharding.

@dignifiedquire I think you missed unixfs extraction?

@hsanjuan

This comment has been minimized.

Contributor

hsanjuan commented Apr 23, 2018

@dignifiedquire the process I've been following is:

  • Create new repos
  • Import history from go-ipfs
  • Add Readmes, license, makefiles, travis, gx
  • Publish gx
  • Then make the PR to go-ipfs
@dignifiedquire

This comment has been minimized.

Member

dignifiedquire commented Apr 23, 2018

@hsanjuan yeah I have done all of that, except for the push to github and publish part, as I wanted feedback on the new repo names & granularity first.

Re unixfs, it wasn't necessary to make this round of extraction work, so I was leaving that for another round

@dignifiedquire

This comment has been minimized.

Member

dignifiedquire commented Apr 23, 2018

I have created and pushed the code for the packages to

Any thoughts what to do about verifcid? How long is the proper fix expected to take until it is propagated up?

@hsanjuan

This comment has been minimized.

Contributor

hsanjuan commented Apr 23, 2018

Re unixfs, it wasn't necessary to make this round of extraction work, so I was leaving that for another round

Ah yes, you're right. For the next one :)

@hsanjuan

This comment has been minimized.

Contributor

hsanjuan commented May 4, 2018

Bumping this. The problem with not merging extractions rapidly is that it becomes very time consuming to maintain the new repos in sync.

I would extract verify-cid temporally as a way to unblock everyone depending on this..

@dignifiedquire

This comment has been minimized.

Member

dignifiedquire commented Aug 1, 2018

closing, as this work was done in other places

@wafflebot wafflebot bot removed the in progress label Aug 1, 2018

@hsanjuan hsanjuan deleted the extract-merkledag-blockservice-path branch Aug 1, 2018

@hsanjuan hsanjuan restored the extract-merkledag-blockservice-path branch Aug 1, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment