Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Fixes #310 - install Missing Analysis Tool. Better feedback would be nice #375

Merged
merged 5 commits into from
Jul 23, 2016
Merged

Fixes #310 - install Missing Analysis Tool. Better feedback would be nice #375

merged 5 commits into from
Jul 23, 2016

Conversation

benclarkwood
Copy link
Contributor

This commit causes goInstallTools to log very differently than present.

Presently, goInstallTools just writes the output of stdout and stderr to the Go outputChannel.

This commit proposes logging much less in the case of success, and only logging the cause of errors.

Let me know what you think @lukehoban.

(A total success)

Installing 9 missing tools
  gorename
  gopkgs
  gocode
  goreturns
  godef
  golint
  go-outline
  go-symbols
  guru

Installing gopkgs SUCCEEDED
Installing go-outline SUCCEEDED
Installing go-symbols SUCCEEDED
Installing godef SUCCEEDED
Installing gorename SUCCEEDED
Installing golint SUCCEEDED
Installing goreturns SUCCEEDED
Installing gocode SUCCEEDED
Installing guru SUCCEEDED

All tools successfully installed. You're ready to Go :).

(A failure)

Installing 9 missing tools
  gorename
  gopkgs
  gocode
  goreturns
  godef
  golint
  go-outline
  go-symbols
  guru

Installing go-outline FAILED
Installing gorename FAILED
Installing gopkgs FAILED
Installing go-symbols FAILED
Installing golint FAILED
Installing goreturns FAILED
Installing gocode FAILED
Installing godef FAILED
Installing guru FAILED

9 tools failed to install.

gorename:
Error: Command failed: go get -u -v golang.org/x/tools/cmd/gorename
Fetching https://golang.org/x/tools/cmd/gorename?go-get=1
https fetch failed: Get https://golang.org/x/tools/cmd/gorename?go-get=1: dial tcp: lookup golang.org: no such host
golang.org/x/tools (download)
# cd /Users/ben/Documents/gowork/src/golang.org/x/tools; git pull --ff-only
fatal: unable to access 'https://go.googlesource.com/tools/': Could not resolve host: go.googlesource.com
package golang.org/x/tools/cmd/gorename: exit status 1
Fetching https://golang.org/x/tools/cmd/gorename?go-get=1
https fetch failed: Get https://golang.org/x/tools/cmd/gorename?go-get=1: dial tcp: lookup golang.org: no such host
golang.org/x/tools (download)
# cd /Users/ben/Documents/gowork/src/golang.org/x/tools; git pull --ff-only
fatal: unable to access 'https://go.googlesource.com/tools/': Could not resolve host: go.googlesource.com
package golang.org/x/tools/cmd/gorename: exit status 1

NOTE: Didn't include every failure's output as they're all the same (no internet connection).

Ben added 2 commits July 5, 2016 15:51
…a summary of the run (e.g. All success, X failures), and if there are failures, print the errors from those failures.
@msftclas
Copy link

msftclas commented Jul 5, 2016

Hi @benclarkwood, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@msftclas
Copy link

msftclas commented Jul 5, 2016

@benclarkwood, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;


outputChannel.appendLine(''); // Blank line for spacing.

Promise.all(missing.map(tool => new Promise<string>((resolve, reject) => {
Copy link
Contributor

@lukehoban lukehoban Jul 16, 2016

Choose a reason for hiding this comment

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

One thing I was meaning to look into/change was running these serially instead of in parallel. I'm pretty sure go get has issues when multiple gets are run in parallel, and users have reported that the existing extension occasionally runs into errors. Since you are making changes here - may make sense to also just change this to do the gets serially?

@lukehoban
Copy link
Contributor

This looks great - big improvement! Left a few comments on the code changes.

@lukehoban lukehoban merged commit 727d157 into microsoft:master Jul 23, 2016
@lukehoban
Copy link
Contributor

I've merged this along with a few additional improvements that address the issues I mentioned in review and fixed a few additional related issues. Thanks again!

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.

3 participants