-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add validation for draft annotations #71
Conversation
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!
annotations/annotations_api.go
Outdated
@@ -68,13 +69,14 @@ type UPPAnnotationsAPI struct { | |||
endpointTemplate string | |||
username string | |||
password string | |||
httpClient *http.Client | |||
httpClient *http.Client `` |
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.
question
: Are the ticks "``" some kind of leftover here, or they do something I don't know ?
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.
It is some kind of leftover :)
handler/handler_test.go
Outdated
|
||
r.Patch("/drafts/content/:uuid/annotations/:cuuid", h.ReplaceAnnotation) | ||
r.HandleFunc("/drafts/content/{uuid}/annotations/{cuuid}", h.ReplaceAnnotation).Methods("PATCH") |
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.
nitpick
(non-blocking): I really like to use the http constants when using the Methods functionality, it's just beautiful.
r.HandleFunc("/drafts/content/{uuid}/annotations/{cuuid}", h.ReplaceAnnotation).Methods("PATCH") | |
r.HandleFunc("/drafts/content/{uuid}/annotations/{cuuid}", h.ReplaceAnnotation).Methods(http.MethodPatch) |
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.
Done
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
Description
What
Why
JIRA Ticket UPPSF-4927
Anything, in particular, you'd like to highlight to reviewers
Mention here sections of code which you would like reviewers to pay extra attention to .E.g
Would appreciate a second pair of eyes on the test
I am not quite sure how this bit works
Is there a better library for doing x
Scope and particulars of this PR (Please tick all that apply)
DoD - Ensure all relevant tasks are completed before marking this PR as "Ready for review"
This Pull Request follows the rules described in our Pull Requests Guide