improve godoc; replace Loggers with embeded fields; refactor Ctx api #673
Conversation
context.go
Outdated
| GOPATH := getEnv(env, "GOPATH") | ||
| if GOPATH == "" { | ||
| GOPATH = defaultGOPATH() | ||
| func (c *Ctx) SetGOPATH(gopath string) error { |
There was a problem hiding this comment.
All other things being equal, I think I'd rather keep this as a constructor. The shape of the existing API more clearly suggests that the GOPATH should really only be set at initialization time, and not modified later.
Is there a functional reason for this change that I'm missing?
There was a problem hiding this comment.
It seemed like it was left in an unusual form as an artifact of old behavior, with the GOPATH logic being the only real work left. In particular, having parameters to pass values straight through to exported fields on the returned struct seems like odd boilerplate - and it was going to compound without Loggers.
Would a better name (Init), or better documentation make usage more clear? Alternatively, the GOPATH portion I've isolated could be encapsulated in a helper function (goodFnName(gopath, wd string) (gopath, gopaths string, err error)), so that NewContext can just set fields.
There was a problem hiding this comment.
Mmm, ok - given that my objection is about a suggestion to the user, I think that as long as the documentation clearly cautions the user appropriately, then it's OK.
There was a problem hiding this comment.
Added another iteration with more documentation.
| // Use of this source code is governed by a BSD-style | ||
| // license that can be found in the LICENSE file. | ||
|
|
||
| // Package dep is a prototype dependency management library. |
There was a problem hiding this comment.
heh, yeah, we need to improve all of these
|
rebase? |
|
sorry, needs another rebase :/ |
#672
This PR improves documentation, and refactors the API around
Ctx, primarily by dissolving theLoggersstruct and embedding the fields directly.