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

Proposal: add context import path upgrade to go tool fix #17040

Closed
SamWhited opened this Issue Sep 9, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@SamWhited
Member

SamWhited commented Sep 9, 2016

It would be convenient if go tool fix had a rule (possibly off by default?) added to upgrade imports of the context package to the new version in the standard library.

Here is a CL that I'm using to upgrade my projects: https://go-review.googlesource.com/#/c/28872/

This makes the new rewrites list read as follows:

Available rewrites are:

context
        Change imports of golang.org/x/net/context to context

gotypes
        Change imports of golang.org/x/tools/go/{exact,types} to go/{constant,types}

netipv6zone
        Adapt element key to IPAddr, UDPAddr or TCPAddr composite literals.

        https://codereview.appspot.com/6849045/

printerconfig
        Add element keys to Config composite literals.

This isn't strictly the same as some of the other rewrites in the tool because the old context package is still perfectly usable, so maybe it would be best to leave this fix off by default and allow it to be manually specified.

@gopherbot

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot commented Sep 9, 2016

CL https://golang.org/cl/28872 mentions this issue.

@quentinmit

This comment has been minimized.

Show comment
Hide comment
@quentinmit

quentinmit Sep 12, 2016

Contributor

You likely want to remain with the old package for quite a while to support building with Go <1.7. I think perhaps we'd want to do this a year or more from now.

Contributor

quentinmit commented Sep 12, 2016

You likely want to remain with the old package for quite a while to support building with Go <1.7. I think perhaps we'd want to do this a year or more from now.

@quentinmit quentinmit added this to the Go1.8Maybe milestone Sep 12, 2016

@SamWhited

This comment has been minimized.

Show comment
Hide comment
@SamWhited

SamWhited Sep 12, 2016

Member

Agreed, having it off by default seems sensible, but going ahead and putting it in the tool seems like it would be nice for those who explicitly want to upgrade.

Member

SamWhited commented Sep 12, 2016

Agreed, having it off by default seems sensible, but going ahead and putting it in the tool seems like it would be nice for those who explicitly want to upgrade.

@SamWhited

This comment has been minimized.

Show comment
Hide comment
@SamWhited

SamWhited Sep 12, 2016

Member

I've updated the CL to require -force context for the fix to be run (it looks like force wasn't otherwise used). Would this be an acceptible way to do it? The behavior could then be changed after the 1.8 release cycle or whenever the Go team decides it's more appropriate to be default-on.

Member

SamWhited commented Sep 12, 2016

I've updated the CL to require -force context for the fix to be run (it looks like force wasn't otherwise used). Would this be an acceptible way to do it? The behavior could then be changed after the 1.8 release cycle or whenever the Go team decides it's more appropriate to be default-on.

@adg

This comment has been minimized.

Show comment
Hide comment
@adg

adg Sep 12, 2016

Contributor
Contributor

adg commented Sep 12, 2016

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Sep 12, 2016

Member

We still ship and use go tool fix? Flashback!

Member

bradfitz commented Sep 12, 2016

We still ship and use go tool fix? Flashback!

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Sep 12, 2016

Member

But yeah, SGTM. Opt-in sounds good.

Member

bradfitz commented Sep 12, 2016

But yeah, SGTM. Opt-in sounds good.

@gopherbot gopherbot closed this in 22d3bf1 Sep 15, 2016

@golang golang locked and limited conversation to collaborators Sep 15, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.