Integrate Sustainable Views draft annotations #70
Conversation
| @@ -228,6 +235,7 @@ func TestWriteMissingTID(t *testing.T) { | |||
| } | |||
| } | |||
|
|
|||
| // nolint:all | |||
There was a problem hiding this comment.
question: Why the nolint: all here?
There was a problem hiding this comment.
There was a problem hiding this comment.
You can place _ for unused parameters and the linter will be happy.
There was a problem hiding this comment.
Correct, I will update the tests in the next PR.
| if err != nil { | ||
| handleWriteErrors("Error decoding request body", err, writeLog, w, http.StatusBadRequest) | ||
| return | ||
| } | ||
|
|
||
| if !mapper.IsValidPACPredicate(addedAnnotation.Predicate) { | ||
| addedAnnotation := addedAnnotationBody["annotation"].(map[string]interface{}) |
There was a problem hiding this comment.
question: Throughout the code there are many places where we are using type assertion without checking if it was successful, we are doing this because we are aware of what is contained in the interface, however this makes the program less panic proof. Do you think we should handle type assertion checks throughout the code?
There was a problem hiding this comment.
Yes you are correct. In theory we should handle type assertion checks throughout the code. However with the next PR we would place validation on the incoming requests, which should minimise the risk of panics. Therefore I have decided not to do so many type assertion checks to make the code more readable/clean.
Yes we are preparing the service for future custom fields in the new types of annotations. |
asparuhft
force-pushed
the
feature/UPPSF-4920
branch
from
124c6ae
to
4730cc1
Compare
Co-authored-by: Manoel Milchev <84855838+ManoelMilchev@users.noreply.github.com>
Description
What
Integrate Sustainable Views draft annotations
Why
JIRA Ticket UPPSF-4920
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