Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Move JS interface definition to a subdirectory #205

Merged
merged 1 commit into from
Jan 25, 2018
Merged

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Jan 22, 2018

This is in preparation for #66 / ipfs/kubo#4498.

  • Why not {src,test}/{js,go}
    • Go usually has the test files in one directory together with the sources
    • Go import path would look weird: github.com/ipfs/interface-ipfs-core/src/go/.. vs github.com/ipfs/interface-ipfs-core/go/

@ghost ghost assigned magik6k Jan 22, 2018
@ghost ghost added the in progress label Jan 22, 2018
@daviddias
Copy link
Contributor

@magik6k sounds good to me. Can you also update the README?

@magik6k
Copy link
Contributor Author

magik6k commented Jan 22, 2018

@diasdavid done (assuming that's what you meant)

README.md Outdated
@@ -102,6 +102,11 @@ In order to be considered "valid", an IPFS core implementation must expose the
- [Miscellaneous](/SPEC/MISCELLANEOUS.md)
- [config](/SPEC/CONFIG.md)

Language-specific interface definitions and tests are in their directories:
Copy link
Contributor

Choose a reason for hiding this comment

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

You only find the tests on the js/src folder. The interface definitions remain at /SPEC

@daviddias
Copy link
Contributor

Can we have at least one of the go-ipfs core APIs spec'ed and tested out just to see how it feels?

@magik6k
Copy link
Contributor Author

magik6k commented Jan 22, 2018

Can we have at least one of the go-ipfs core APIs spec'ed and tested out just to see how it feels?

For the code the plan is to just move https://github.com/ipfs/go-ipfs/tree/master/core/coreapi/interface here into go/. Written specs will probably just be docs from the interface code C-c C-v'd into /SPECS docs.

I removed the readme change as it didn't really make sense.

@ghost
Copy link

ghost commented Jan 23, 2018

I'm not sure how well gx deals with packages in repo subdirectories. If it turns out that we have to perform in-depth surgery on gx, I'd recommend putting the Go interfaces into their own repo.

Could you try that out before we merge this PR?

@magik6k
Copy link
Contributor Author

magik6k commented Jan 23, 2018

So I did gx init in $GOPATH/github.com/ipfs/interface-ipfs-core/go, it worked fine without any hacks, though only go/ directory was pushed to gx. This is go/package.json I got:

{
  [...]
  "bugs": {
    "url": "https://github.com/ipfs/interface-ipfs-core"
  },
  "gx": {
    "dvcsimport": "github.com/ipfs/interface-ipfs-core/go"
  },
  "gxVersion": "0.11.0",
  [...]
  "name": "go",
}

Go tooling didn't have any problems with importing this package (though if a user wants to go-get it it needs to be github.com/ipfs/interface-ipfs-core/go (note the /go)).

I'd say that it works well enough to have these things in one repo

@daviddias
Copy link
Contributor

Go tooling didn't have any problems with importing this package (though if a user wants to go-get it it needs to be github.com/ipfs/interface-ipfs-core/go (note the /go)).

That's excellent! Thank you @magik6k ❤️

Mind rebasing master onto this PR to fix merge conflicts?

@magik6k
Copy link
Contributor Author

magik6k commented Jan 25, 2018

@diasdavid done

@daviddias daviddias merged commit d20afe3 into master Jan 25, 2018
@daviddias daviddias deleted the feat/js-directory branch January 25, 2018 23:08
@ghost ghost removed the in progress label Jan 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants