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.

Sign up for GitHub

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

Jump to bottom

x/tools/gopls: change behavior of dependency upgrade suggestions #38339

Closed
stamblerre opened this issue Apr 9, 2020 · 13 comments
Closed

x/tools/gopls: change behavior of dependency upgrade suggestions #38339

stamblerre opened this issue Apr 9, 2020 · 13 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
gopls/v0.6.5

Comments

@stamblerre
Copy link
Contributor

Some users have mentioned that the upgrade code lenses are a little invasive because they are shown per-line. Here are some general thoughts / ideas for how to handle this:

  • Don't show upgrades for pseudoversions? Or only for pseudoversions older than some amount of time (1 month? more?).
  • Offer upgrades as code actions instead of code lenses. The "upgrade all" could still be a code lens, but this way we could offer more granular updates. Someone may not want to update to a new major version, but they may want to update to a new minor or patch release - code actions give more options.
@stamblerre stamblerre added this to the gopls/v0.5.0 milestone Apr 9, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Apr 9, 2020
@hyangah
Copy link
Contributor

hyangah commented Apr 10, 2020

Or is there any way to disable some of the code lenses?
As we add more code lenses, we will have more diverse opinions about them.

@stamblerre
Copy link
Contributor Author

Or is there any way to disable some of the code lenses?

There is a "editor.codeLens" setting, but I don't think you can configure it per-code lens.

@hyangah hyangah mentioned this issue Apr 16, 2020
Closed
@stamblerre stamblerre mentioned this issue Apr 20, 2020
Closed
@stamblerre stamblerre modified the milestones: gopls/v0.5.0, gopls/v1.0.0 Jul 22, 2020
@stamblerre
Copy link
Contributor Author

stamblerre commented Sep 9, 2020 •
edited
Loading

We now have per-code lens configuration.

While I would like to eventually make improvements to the behavior of the upgrade code lenses, I don't think it's necessary for v1.0.

@stamblerre stamblerre removed this from the gopls/v1.0.0 milestone Sep 9, 2020
@stamblerre stamblerre added this to the gopls/unplanned milestone Oct 21, 2020
@stamblerre
Copy link
Contributor Author

https://golang.org/cl/271297 removed the per-require upgrade code lenses, so we should prioritize adding the upgrades as diagnostics paired with suggested fixes. I think they should be treated similarly to how fillstruct works in Go files--we don't want to use a visible underlined diagnostic since it may bother the user visually (if they specifically chose a lower version), but when they click on the require, they should be prompted to upgrade it.

/cc @heschik

@stamblerre stamblerre mentioned this issue Dec 3, 2020
Closed
@nopyhe
Copy link

nopyhe commented Dec 4, 2020

It's not a good idea to drop per-dependency update notification when the alternative method not ready. I also have to stay on gopls v0.5.3.

@stamblerre
Copy link
Contributor Author

The "Upgrade all dependencies" code lens still appears at the top of the go.mod file. We will be improving the UX of the per-dependency upgrade feature, and it was removed for the reasons described in the release notes.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/275432 mentions this issue: internal/lsp/mod: add an "Upgrade direct dependencies" code lens

@nopyhe
Copy link

nopyhe commented Dec 5, 2020

@stamblerre Can we have a code lens like "Check for upgrade" to trigger upgrade suggestions for each require in a go.mod file as before.
I don't want to click every require to check whether it can be upgraded or not when I need to upgrade some of them.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Dec 6, 2020
@0-wiz-0
gopls: update to 0.5.5.
d5e1c1d 
0.5.5

This is a patch release to fix two bugs in gopls/v0.5.4.

Fixes

Excessive reloading of packages outside of GOPATH or a module

File corruption with CRLF line endings and //-style comments

golang/go#42646 was supposed to have been fixed in gopls/v0.5.4,
but it was not. golang/go#42923 was reported and fixed.

0.5.4

Features

Opening a project that contains a module in a subdirectory

Previously, gopls required that you open your editor exactly at or
below the module root (the directory containing the go.mod). Now,
you can open a directory that contains exactly one module in a
subdirectory, and gopls will work as expected. For details on
multi-module workspaces, see below.

Removal of the granular go.mod upgrade codelenses

Previously, we offered a code lens to suggest upgrades for each
require in a go.mod file. In anticipation of changes that limit
the amount that gopls accesses the network, we have decided to
remove and reevaluate this feature. Users had mentioned that the
code lenses cluttered their go.mod files, especially if they didn't
actually want to upgrade. golang/go#38339 tracks the work to revamp
this feature. An "Upgrade all dependencies" code lens should still
appear at the top of your go.mod file.

Improved error message reports

Previously, critical error messages were reported as message pop-up
that would re-trigger as you type. Many users would find this
annoying. We have changed the approach to show error messages as
progress reports, which should be less intrusive and appear more
like status bars.

Improved memory usage for workspaces with multiple folders

We are now using a coarser cache key for package type information.
If you use the gopls daemon, this may reduce your total memory
usage.

Experimental

Multi-module workspace support

The proposal described in golang/go#32394 is still in development
and off by default. See our progress by tracking the multi-module
workspace milestone and project.

Enable multi-module workspace support by adding the following to
your settings:

"gopls": { "experimentalWorkspaceModule": true, }

With this setting, you will be able to open a directory that contains
multiple modules or a directory that contains nested modules.

Give this a try if you're interested in this new feature, but please
note that it is still very experimental. Please file issues if you
encounter bugs.

Fixes

File corruption with CRLF line endings and /**/-style comments

Previously, when you organized the imports in a file with CRLF line
endings and multi-line comments, the formatter might output incorrect
content for the file, rendering it invalid Go code. This issue has
popped up a number of times, but we believe it has finally been
fixed for good. If you are using Windows with CRLF line ending,
please report any regressions. For full details, see golang/go#42646.
@stamblerre
Copy link
Contributor Author

Thanks for the suggestion--we will consider that option. Would the "upgrade all direct dependencies" code lens work for you?

@nopyhe
Copy link

nopyhe commented Dec 7, 2020 •
edited
Loading

@stamblerre Not really. I want to keep my dependencies up to date. But there are some reasons I can't upgrade all of them. For example, some dependencies have a breaking change and it needs some time to handle, which is not the top priority.
In another word, what I want is a convenient way to check the latest version info of dependencies, upgrade most of them and keep some dependencies alone.

gopherbot pushed a commit to golang/tools that referenced this issue Dec 8, 2020
@stamblerre
internal/lsp/mod: add an "Upgrade direct dependencies" code lens
5737bec 
`go get -u all` can have unexpected behavior for some users, since
upgrades all transitive dependencies as well as direct ones. Offer users
the ability to upgrade only direct dependencies via a code lens.

Also, change the placement of the upgrade code lenses. The module
declaration was getting cluttered, and they make more sense above the
requires anyway.

Updates golang/go#38339

Change-Id: I6790a02f2a334f3f4d1d89934b2c4dc4f11845ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/275432
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@stamblerre
Copy link
Contributor Author

That's fair, but it should still make it a little simpler to upgrade all dependencies and then revert the ones you don't want to upgrade. I think the per-require code action will work well for most cases, because it will allow us to offer multiple versions, but I do also like the idea of a "check for updates" command that we could expose as a code lens.

@xiegeo
Copy link

xiegeo commented Dec 19, 2020

I would like to add a user experience report.

I don't find the per line code lenses invasive. In fact, it's the most useful feature. I only goto the go.mod file to check for potential upgrades.

I work on some projects that suffer from upstream breaking Semitic Versioning, but if go tooling makes keeping certain versions back easier, more developers will feel freer to break SV. This will end up making the development experience worth.

"but it should still make it a little simpler to upgrade all dependencies and then revert the ones you don't want to upgrade. "
Unfortunately, not. Upgrade all can error out with compatibility errors. It's more "I can't" than "I won't".

If "Upgrade direct dependencies" is a useful workflow, that should be added to the command line tooling first instead as a gopls only feature.

marwan-at-work pushed a commit to marwan-at-work/tools that referenced this issue Dec 23, 2020
@stamblerre @marwan-at-work
internal/lsp/mod: add an "Upgrade direct dependencies" code lens
e0f7517 
`go get -u all` can have unexpected behavior for some users, since
upgrades all transitive dependencies as well as direct ones. Offer users
the ability to upgrade only direct dependencies via a code lens.

Also, change the placement of the upgrade code lenses. The module
declaration was getting cluttered, and they make more sense above the
requires anyway.

Updates golang/go#38339

Change-Id: I6790a02f2a334f3f4d1d89934b2c4dc4f11845ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/275432
Trust: Rebecca Stambler <rstambler@golang.org>
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
gopls-CI: kokoro <noreply+kokoro@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
@stamblerre
Copy link
Contributor Author

We reworked the user experience in gopls/v0.6.5, and you can see an explanation of the new workflow in the release notes. Closing this issue as a result.

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.6.5 Feb 10, 2021
@golang golang locked and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
gopls/v0.6.5
Development

No branches or pull requests
5 participants
@xiegeo @hyangah @stamblerre @nopyhe @gopherbot