Repository navigation
define struct types instead of using anonymous structs - #1900
Conversation
The types of the fields of the EditChange struct were defined inline as anonymous structs. This made it tedious to use the EditChange struct in testcases, because the anonymous type needed to be redefined (incl the exact same JSON tags) whenever it was assigned. The TestEdit* testcases are good example for it. Using anonymous struct also does not scale well when the struct changes. For example when a field is added to an anonymous struct, every place where the anonymous struct is assigned must also be changed. Another advantage of using regular types is that accessors will be generated automatically and can be used instead of having to manually check for nil values when accessing the struct fields. Define the types as regular structs in event_types.go instead.
Codecov Report
@@ Coverage Diff @@
## master #1900 +/- ##
=======================================
Coverage 97.65% 97.65%
=======================================
Files 105 105
Lines 6786 6786
=======================================
Hits 6627 6627
Misses 86 86
Partials 73 73
Continue to review full report at Codecov.
|
gmlewis
left a comment
There was a problem hiding this comment.
While you are here, do you want to do the same thing for:
ProjectChangeProjectCardChangeProjectColumnChangeTeamChangeRepositoryVulnerabilityAlertEvent
?
Co-authored-by: Glenn Lewis <6598971+gmlewis@users.noreply.github.com>
|
@gmlewis I also replaced the other anonymous struct types. |
|
Also, @fho - could you please remove the "Draft" setting when you are ready? |
|
@gmlewis I'll remove the draft status as soon as my employer signed the CA, probably beginning of next week. I only noticed that it was missing after I created it. :-) |
Interesting - the googlebot marked the PR as having passed the CLA test. OK, sounds good. |
It's because I signed the CA for myself already in the past. |
|
@gmlewis my employer signed the CLA now also. Update: now it shows up, yesterday was still one signature missing |
|
Friendly ping, @wesleimp |
|
If we are expecting to replace all anonymous structs with defined struct types, then I guess a few are still missing in this PR.
|
|
@sagar23sj thanks, I'll update the PR and also replace those anonymous structs. It was not my initial intent of the PR to replace all anonymous structs but it makes sense to have a consistent solution. |
…ucts Fix wrong struct name in godoc comment
This fixes the following golint warning: update-urls/main.go:658:1: receiver name ep should be consistent with previous receiver name e for Endpoint
|
done |
|
@fho - would you like to resolve the merge conflicts, or would you like me to do that? I'm fine either way. |
|
@gmlewis also both fine with me, feel free to do changes :-) |
Some fields were defined inline as anonymous structs.
This made it tedious to use these structs in testcases, because
the anonymous type needed to be redefined (incl the exact same JSON
tags) whenever it was assigned.
Using anonymous struct also does not scale well when the struct changes.
For example when a field is added to an anonymous struct, every place
where the anonymous struct is assigned must also be changed.
Another advantage of using regular types is that accessors will be
generated automatically and can be used instead of having to manually
check for nil values when accessing the struct fields.
Define the types as regular structs instead.
PR Status: PR is finished but I noticed that my company hasn't signed the CA yet. As soon as it is done, I'll transition the PR to "ready for review".