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

all: goimports -w . #14788

Merged
merged 1 commit into from
Nov 18, 2022
Merged

all: goimports -w . #14788

merged 1 commit into from
Nov 18, 2022

Conversation

sashamelentyev
Copy link
Contributor

Signed-off-by: Sasha Melentyev sasha@melentyev.io

@sashamelentyev sashamelentyev force-pushed the imports branch 2 times, most recently from 23ca390 to 53e79b9 Compare November 16, 2022 19:28
@@ -26,7 +26,7 @@ import (
"go.etcd.io/etcd/client/v3"
integration2 "go.etcd.io/etcd/tests/v3/framework/integration"
"go.etcd.io/etcd/tests/v3/framework/testutils"
clientv3test "go.etcd.io/etcd/tests/v3/integration/clientv3"
"go.etcd.io/etcd/tests/v3/integration/clientv3"
Copy link
Member

Choose a reason for hiding this comment

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

It's updated by goimports -w automatically? Why did previous file add clientv3test, while this file remove clientv3test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

goimports -w add this:

	pb "go.etcd.io/etcd/api/v3/etcdserverpb"
	"go.etcd.io/etcd/client/pkg/v3/transport"
	integration2 "go.etcd.io/etcd/tests/v3/framework/integration"
	"go.etcd.io/etcd/tests/v3/framework/testutils"
	"go.etcd.io/etcd/tests/v3/integration/clientv3"
	clientv3test "go.etcd.io/etcd/tests/v3/integration/clientv3"
	"google.golang.org/grpc"

Mb I remove changes in tests?

Copy link
Contributor

Choose a reason for hiding this comment

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

We want to keep

clientv3test "go.etcd.io/etcd/tests/v3/integration/clientv3"

And change in this file goes in the other direction. So it's why we are surprised.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

I add clientv3test "go.etcd.io/etcd/tests/v3/integration/clientv3"myself and run goimports and all ok. Rerun goimports have no changes

Signed-off-by: Sasha Melentyev <sasha@melentyev.io>
@ptabor
Copy link
Contributor

ptabor commented Nov 17, 2022

Please add it to:
https://github.com/etcd-io/etcd/blob/main/scripts/fix.sh

as this change makes sense if we want to keep it consistent going forward.

And the benefit of this change is that there is less 'surprise' that the /client/v3 package is not addressable by v3.

@sashamelentyev
Copy link
Contributor Author

Please add it to: https://github.com/etcd-io/etcd/blob/main/scripts/fix.sh

as this change makes sense if we want to keep it consistent going forward.

And the benefit of this change is that there is less 'surprise' that the /client/v3 package is not addressable by v3.

After goimports -w . have changes in *.pb.go files...

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you @sashamelentyev

Please consider to resolve #14788 (comment) suggested by @ptabor , of course in a separate PR.

@ahrtr ahrtr merged commit 6d8139d into etcd-io:main Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants