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

Remove consul-api usage in favor of official consul/api #8794

Merged
merged 1 commit into from
Sep 13, 2021

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Sep 9, 2021

Signed-off-by: Andrew Mason [email protected]

Description

This is a subset of the code changes made in #8786, specifically the changes to go/vt/orchestrator so that we can remove armon/consul-api as a dependency.

This is a follow-up to #8784.

Related Issue(s)

#8785

Checklist

  • Should this PR be backported?
  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

@mattlord
Copy link
Contributor

mattlord commented Sep 9, 2021

Just noting that the reason the usage of this was replaced is due to the linter warnings/suggestions that occur when committing the import change within the file:

Linting go/vt/orchestrator/kv
       3 suggestions for:
go run golang.org/x/lint/golint go/vt/orchestrator/kv/consul.go

Lint suggestions were found. They're not enforced, but we're pausing
to let you know before they get clobbered in the scrollback buffer.

Press enter to cancel, "s" to step through the warnings or type "ack" to continue: s

go/vt/orchestrator/kv/consul.go:68:1: receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"
go/vt/orchestrator/kv/consul.go:77:1: receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"
go/vt/orchestrator/kv/consul.go:91:1: receiver name should be a reflection of its identity; don't use generic names such as "this" or "self"

The warning/suggestion comes from the golang best practice noted here: https://github.com/golang/go/wiki/CodeReviewComments#receiver-names

We could just leave that usage in go/vt/orchestrator/kv/consul.go alone since it looks like orchestrator uses this as the method receiver name all over the place.

@harshit-gangal harshit-gangal merged commit d46cc63 into vitessio:main Sep 13, 2021
@ajm188 ajm188 deleted the am_consul_dep_part_2 branch September 13, 2021 14:05
@mattlord
Copy link
Contributor

Split from #8784

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants