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

x/tools/cmd/goimports: put std, cmd packages in separate blocks #38735

Closed
jayconrod opened this issue Apr 28, 2020 · 4 comments
Closed

x/tools/cmd/goimports: put std, cmd packages in separate blocks #38735

jayconrod opened this issue Apr 28, 2020 · 4 comments

Comments

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Apr 28, 2020

This came up in CL 228378 and many others. If we delete all the imports from src/cmd/go/internal/modload/modfile.go then run goimports, we should see cmd imports in a separate group from std imports:

import (
	"fmt"
	"io/ioutil"
	"path/filepath"

	"cmd/go/internal/base"
	"cmd/go/internal/cfg"
	"cmd/go/internal/modfetch"

	"golang.org/x/mod/modfile"
	"golang.org/x/mod/module"
	"golang.org/x/mod/semver"
)

Instead, all the std and cmd imports are grouped together.

import (
	"cmd/go/internal/base"
	"cmd/go/internal/cfg"
	"cmd/go/internal/modfetch"
	"fmt"
	"io/ioutil"
	"path/filepath"

	"golang.org/x/mod/modfile"
	"golang.org/x/mod/module"
	"golang.org/x/mod/semver"
)

goimports does the right thing if at least one cmd import is already in a separate group. It combines the groups when adding the first std or cmd import though.

@heschik
Copy link
Contributor

@heschik heschik commented Apr 29, 2020

To be clear, do you really want it in three sections, or do you just want cmd/ grouped with golang.org/x? Because the latter seems like a bug that should be fixed. The former would be more of a job for -local.

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 29, 2020

The existing files in cmd/go mostly have it in three sections, so we would like to retain that.

But it looks like the existing files in cmd/compile make no such distinction. Perhaps we should reformat cmd to eliminate it too, and collapse down to only two sections.

@heschik
Copy link
Contributor

@heschik heschik commented Apr 29, 2020

Not sure what you'd like me to do here. I can change it so that cmd/ is grouped with golang.org, but -local is the only way to get three groups, and in general goimports will never merge groups, so if you want to collapse down to two you're going to have to give it some help.

@jayconrod
Copy link
Contributor Author

@jayconrod jayconrod commented Apr 30, 2020

After some discussion, we're leaning toward just combining std and cmd sections.

I was not at all aware of the -local flag, so it looks like that's there if we decide otherwise.

I'll close this. Looks like there's nothing to do on the goimports side.

@jayconrod jayconrod closed this Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.