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

Fix Add Import action by using go list to suggest import paths #259

Closed
wants to merge 1 commit into from

Conversation

marwan-at-work
Copy link
Contributor

@marwan-at-work marwan-at-work commented Jun 25, 2020

Fixes #258

@googlebot googlebot added the cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change. label Jun 25, 2020
@gopherbot
Copy link
Collaborator

This PR (HEAD: 17b0c21) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/vscode-go/+/240001 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Collaborator

Message from Rebecca Stambler:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/240001.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Collaborator

Message from Marwan Sulaiman:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/240001.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Collaborator

Message from Rebecca Stambler:

Patch Set 1:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/240001.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Collaborator

This PR (HEAD: 9e05bfd) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/vscode-go/+/240001 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

@gopherbot
Copy link
Collaborator

Message from Marwan Sulaiman:

Patch Set 2:

(1 comment)


Please don’t reply on this GitHub thread. Visit golang.org/cl/240001.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Collaborator

Message from Hyang-Ah Hana Kim:

Patch Set 2:

(3 comments)

I observed this CL caused multiple test failures - https://github.com/golang/vscode-go/pull/259/checks?check_run_id=809087856

Looking into a bit, go list all triggered from the "test listPackages" test, for example, failed with errors like

'can't load package: package github.com/uudashr/gopkgs: code in directory /Users/hakim/go/src/github.com/uudashr/gopkgs expects import "github.com/uudashr/gopkgs/v2"
can't load package: package github.com/uudashr/gopkgs/internal: code in directory /Users/hakim/go/src/github.com/uudashr/gopkgs/internal expects import "github.com/uudashr/gopkgs/v2/internal"
can't load package: package test/testfixture/importTest: found packages main (cgoImports.go) and hello (groupImports.go) in /Users/hakim/go/src/test/testfixture/importTest'

The detailed errors may vary depending on what's under the current GOPATH directory, but it seems like go list all is more sensitive to what's currently under GOPATH.

Rebecca, have you seen errors like this when working with go list for gopls?
Another question is the version compatibility - is go list working well with earlier versions of go (prior to go1.13)?


Please don’t reply on this GitHub thread. Visit golang.org/cl/240001.
After addressing review feedback, remember to publish your drafts!

@gopherbot
Copy link
Collaborator

Message from Hyang-Ah Hana Kim:

Patch Set 2:

(2 comments)

Ok, I think I figured out why promises didn't seem to resolve.

Still various tests are failing

  • Replace vendor packages with relative path
  • all tests related to Completion.

Would be nice if you can investigate the test failures. Thanks!


Please don’t reply on this GitHub thread. Visit golang.org/cl/240001.
After addressing review feedback, remember to publish your drafts!

@hyangah
Copy link
Contributor

hyangah commented Dec 7, 2021

@marwan-at-work I think this PR is obsolete. Can we close this?

@marwan-at-work
Copy link
Contributor Author

Yep! Closing it now. Thanks for the ping 🏓

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Used by googlebot to label PRs as having a valid CLA. The text of this label should not change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tools: replace use of 'gopkgs' with 'go list all'
4 participants