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

GoDoc and Public API #672

Closed
jmank88 opened this issue May 28, 2017 · 6 comments
Closed

GoDoc and Public API #672

jmank88 opened this issue May 28, 2017 · 6 comments

Comments

@jmank88
Copy link
Collaborator

jmank88 commented May 28, 2017

There are currently many undocumented, exported members in the top level package dep - GoDoc.

Is there a long term plan for this package's public API?
Was there a design to the current state, or were things just made public as needed by commands?

In its current form, some exported members could use simple comments, others could perhaps be made unexported or internal, and some pieces have evolved and could use small API tweaks, even if only to fix things like stale names.

Unless I missed an existing conversation, this issue can be a central place to discuss the GoDoc and Public API, and to link incremental PRs.

I have an incoming PR for general documentation and specific changes to Ctx and Loggers.

@carolynvs
Copy link
Collaborator

Related issues (none that answer this really good question though): #527 #594 #575. I'd really like for us decide what is exported (and documented) for now, even if we go a different direction in a later milestone.

@sdboyer
Copy link
Member

sdboyer commented May 31, 2017

Yeah, we need some real thought around this.

My off-the-cuff thoughts:

  1. dep should primarily concern itself with exporting:
    1. Its concrete representations of gps' required types, e.g. Lock, Manifest, and ProjectAnalyzer
    2. Logic for loading up a "project", as dep defines it
    3. Logic for preparing a gps.SolveParameters in the same way dep does it (absent CLI arguments)
    4. maybe the logic for writing out state (so, the SafeWriter). "maybe" because this is arguably something that we should treat as "the tool is the API", in much the way the go tool does.
  2. dep should avoid exporting anything that's not strictly necessary for the above
  3. gps is already designed and mostly documented as a self-contained API. We should aim to keep it self-contained, expanding it with functionality from dep when there is an operation that is agnostic to any of dep's specific assumptions.

@sdboyer
Copy link
Member

sdboyer commented May 31, 2017

(implicit in the above is that we should re-export gps)

@jmank88
Copy link
Collaborator Author

jmank88 commented Jun 2, 2017

Any thoughts on whether func PruneProject should remain exported?
If exported, then would it make sense as a method? (*Project) Prune(...)
If unexported, then the whole thing can move into cmd/dep/prune.go. I suspect using (*pruneCommand) Run() if possible would be encouraged over calling PruneProject directly anyways.

@sdboyer
Copy link
Member

sdboyer commented Jun 2, 2017

We can unexport it - the plan is to shift that behavior more directly into dep ensure. See #120 (comment).

@sdboyer
Copy link
Member

sdboyer commented Aug 28, 2017

converting this to an epic in zenhub - this is really its own non-trivial cluster of tasks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants