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.

Sign up for GitHub

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

Jump to bottom

Consistently name client interfaces #319

Closed
invidian opened this issue Oct 2, 2020 · 2 comments · Fixed by #345
Closed

Consistently name client interfaces #319

invidian opened this issue Oct 2, 2020 · 2 comments · Fixed by #345

Comments

@invidian
Copy link
Contributor

invidian commented Oct 2, 2020

Expected Behaviour

One can easily switch between working on code around workflows and templates and naming should follow the same convention, so interface names are not surprising, following POLA.

Current Behaviour

  • github.com/tinkerbell/tink/protos/workflow has WorkflowSvcClient
  • github.com/tinkerbell/tink/protos/hardware has HardwareServiceClient
  • github.com/tinkerbell/tink/protos/template has TemplateClient
@mmlb
Copy link
Contributor

mmlb commented Oct 2, 2020

100% yes!

@mmlb mmlb mentioned this issue Oct 19, 2020
Merged
@mmlb
Copy link
Contributor

mmlb commented Oct 20, 2020

Thanks @invidian take a look at #345 which addresses this along with other protobuf related warts I encountered.

@mergify mergify bot closed this as completed in #345 Oct 27, 2020
mergify bot added a commit that referenced this issue Oct 27, 2020
@mergify
Show protos some love (#345)
0b477fd 
## Description

This overhauls and fixes a bunch of warnings/nits in our protobufs. Also, I wasn't able to build the protobufs because of version mismatches and what not.

## Why is this needed

I was not happy with the state of our protobuf build setup, managing protobuf dependencies and general protobuf lint/style issues.

Also,
Fixes: #319 

## How Has This Been Tested?

Builds and `go test ./...` works.


## How are existing users impacted? What migration steps/scripts do we need?

Custom clients using our protobuf files will need to be updated, I don't know of any such clients.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Show protos some love mmlb/tinkerbell-tink
2 participants
@mmlb @invidian