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

Refactor file organization #43

Merged
merged 1 commit into from
Nov 11, 2019
Merged

Conversation

hannahhoward
Copy link
Collaborator

Goals

Reduce future import cycles, and expose more types properly, and expose an actual interface
other implementers can replace.

Implementation

  • Root package is now interface/types file
  • The main implementation is moved into /impl
  • Move some common types in gsmsg (weird place) to the root package

Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

I like this. It makes it clear which types are "public" and which types are internal.

@@ -39,7 +39,7 @@ If your existing library (i.e. `go-ipfs` or `go-filecoin`) uses these other olde

```golang
import (
graphsync "github.com/ipfs/go-graphsync"
graphsync "github.com/ipfs/go-graphsync/impl"
Copy link
Member

Choose a reason for hiding this comment

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

uber-nit: the go community often uses pkg or even internal (inaccessible to other projects) for packages like this.

Copy link
Member

@anorth anorth Nov 10, 2019

Choose a reason for hiding this comment

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

I believe pkg is the dir used for the external packages. Be careful with internal since it's actually enforced (you'll need to put ctors etc outside). See https://github.com/golang-standards/project-layout

LGTM regardless.

To reduce future import cycles, and expose more types properly, move the root package to an
interface/types file and move the main implementation into /impl. This also exposes an interface
other implementers can replace.
@hannahhoward hannahhoward changed the base branch from feat/extensions to master November 11, 2019 21:38
@hannahhoward hannahhoward merged commit 2cbb73f into master Nov 11, 2019
@mvdan mvdan deleted the feat/organization-refactor branch December 15, 2021 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants