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

Simplify findProjectRoot #17

Merged
merged 1 commit into from
Nov 30, 2016
Merged

Simplify findProjectRoot #17

merged 1 commit into from
Nov 30, 2016

Conversation

freeformz
Copy link

This is mostly the non controversial changes from #11, plus a more to
the std lib stub function.

/cc @sdboyer

This is mostly the non controversial changes from #11, plus a more to
the std lib stub function.
Copy link
Member

@sdboyer sdboyer left a comment

Choose a reason for hiding this comment

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

just one nit/question

return nil, fmt.Errorf("could not open %s: %s", mp, err)
if os.IsNotExist(err) {
// TODO: list possible solutions? (dep init, cd $project)
return nil, fmt.Errorf("no %v found in project root %v", manifestName, p.absroot)
Copy link
Member

Choose a reason for hiding this comment

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

nit: is it standard practice to use %v for string placeholders? It works, ofc; I've just always tended to %s.

Copy link
Author

Choose a reason for hiding this comment

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

I'm interested in hearing what @adg says about this, but %v == "default format", so I generally use it for most things unless I need more control over the format.

@freeformz freeformz merged commit b17dca4 into master Nov 30, 2016
@jessfraz jessfraz deleted the findRoot branch December 2, 2016 00:15
zbintliff added a commit to zbintliff/dep that referenced this pull request Mar 3, 2017
krisnova pushed a commit to krisnova/dep that referenced this pull request Apr 21, 2017
Introduce intermediary smcache within the solver
ibrasho pushed a commit to ibrasho-forks/dep that referenced this pull request May 10, 2017
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.

2 participants