apply gofmt, add new fields to ReviewInput #123
Conversation
It fixes up godocs slightly.
In particular, I need ignore_automatic_attention_set_rules, but I'm adding all other missing fields in the struct too. I've added RecipientType as a named type, as it's an enum-like string.
| @@ -360,6 +366,19 @@ type AttentionSetInfo struct { | |||
| Reason string `json:"reason"` | |||
| } | |||
|
|
|||
| type RecipientType string | |||
There was a problem hiding this comment.
As far as I can see, other enum-type strings, we keep as stings.
We don't validate enums here.
What do you think about keeping this as string?
Or maybe: What was the rational behind adding this as its own type?
There was a problem hiding this comment.
Alternatively: Adding https://gerrit-review.googlesource.com/Documentation/user-notify.html#recipient-types as the doc link here might help.
There was a problem hiding this comment.
No strong feeling here. I just personally go for named types when it comes to enums; otherwise it's easy to typo a string and have code silently fail. You can't add the named type later on v1+, because that's technically a breaking change, which is why I added it now. I didn't add the constants because those can always be added later.
I don't mind either way though, so I'm OK with deleting if you disagree. Alternatively, happy for you to apply the suggested change and merge.
There was a problem hiding this comment.
I will go with the named type. Thanks @mvdan!
|
Thanks a lot @mvdan |
(see commit messages - please do not squash)