Skip to content

Use an alias so goimports doesn't delete a real import#3886

Merged
michael-berlin merged 1 commit intovitessio:masterfrom
dweitzman:gofmt
May 2, 2018
Merged

Use an alias so goimports doesn't delete a real import#3886
michael-berlin merged 1 commit intovitessio:masterfrom
dweitzman:gofmt

Conversation

@dweitzman
Copy link
Copy Markdown
Collaborator

goimports gets confused by dashes and dots in import names.

Signed-off-by: David Weitzman dweitzman@pinterest.com

Copy link
Copy Markdown
Contributor

@michael-berlin michael-berlin left a comment

Choose a reason for hiding this comment

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

LGTM after comment about "sort" is addressed.

This is indeed a problem. goimports just silently removes the "ldap.v2" import without the alias. Thank you for fixing this :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I didn't know that this passes the linter now. It seems to be a recent change?

My local goimports does the same thing, so that's fine with me :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Can you please add it to the first group above i.e. next to "sync"?

Thanks!

goimports gets confused by dashes and dots in import names.

Signed-off-by: David Weitzman <dweitzman@pinterest.com>
@dweitzman
Copy link
Copy Markdown
Collaborator Author

Moved the "sort" import

Copy link
Copy Markdown
Contributor

@michael-berlin michael-berlin left a comment

Choose a reason for hiding this comment

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

I'll merge after tests have passed.

@michael-berlin michael-berlin merged commit a3d03d4 into vitessio:master May 2, 2018
@dweitzman dweitzman deleted the gofmt branch May 5, 2018 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants