-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: worker auth endpoints #212
Conversation
Codecov Report
@@ Coverage Diff @@
## main #212 +/- ##
==========================================
+ Coverage 90.75% 91.42% +0.66%
==========================================
Files 18 18
Lines 1331 1365 +34
==========================================
+ Hits 1208 1248 +40
+ Misses 91 87 -4
+ Partials 32 30 -2
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I have one question about why we need some functions, but that doesn't block merging this.
@@ -300,6 +351,24 @@ func ExampleWorkerService_Add() { | |||
fmt.Printf("Received response code %d, for worker %+v", resp.StatusCode, worker) | |||
} | |||
|
|||
func ExampleWorkerService_RefreshAuth() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this used? Same goes for the rest of the ExampleWorkerService_*
funcs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly im not sure. i added it for consistency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seemed like leftover boiler plate tests from the initial worker router commits. we can probably remove them in favor the more detailed tests above them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is used for documentation, see https://pkg.go.dev/github.com/go-vela/[email protected]/vela#example-WorkerService.Update as an example (no pun intended) which is extracted from ExampleWorkerService_Update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh!!! 😮💨 thats awesome. i didnt know go doc used test naming to populate that. thanks @wass3rw3rk
this adds the server functionality introduced in go-vela/server#790
it also cleans up a couple tests