refactor!: Support array type for CustomProperty.DefaultValue field
#3740
Conversation
gmlewis
changed the title
DefaultValue fieldCustomProperty.DefaultValue field
gmlewis
added
NeedsReview
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3740 +/- ##
=======================================
Coverage 91.11% 91.11%
=======================================
Files 187 187
Lines 16697 16697
=======================================
Hits 15213 15213
Misses 1296 1296
Partials 188 188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Thank you, @zyfy29!
One minor tweak, please, and then we should be ready for a second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear
github/orgs_properties.go
Outdated
| // Default value of the property. | ||
| DefaultValue *string `json:"default_value,omitempty"` | ||
| // Default value of the property. Can be null, string or array of strings. | ||
| DefaultValue any `json:"default_value,omitempty"` |
There was a problem hiding this comment.
@zyfy29 - for the a null to be passed through to the backend, we need to remove the omitempty.
Also, please make sure that there is a unit test where the value is nil (unassigned) so we demonstrate that the null is passed through to the backend.
| DefaultValue any `json:"default_value,omitempty"` | |
| DefaultValue any `json:"default_value"` |
|
Oh, sorry, now it should be OK |
There was a problem hiding this comment.
Thank you, @zyfy29!
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
cc: @stevehipwell - @alexandear
BREAKING CHANGE:
CustomProperty.DefaultValuewas*stringbut now isany.Fixes: #3692.
Fixes: #3741.