Closed gcasar closed 2 years ago
@gcasar Thanks for picking this up. I'll review it asap this week.
Regarding the omitnil
proposal you linked, you could check out my other package jettison
. (which support it), but after a quick peek to your custom marshaller implem, I think it's better than introducing a new dependency just for that need (which would only be for JSON marshaling anyway, and not YAML).
@gcasar Sorry for the delay, just got back from holidays. Please see the minor fixes suggestions, otherwise looks good. I'd rather squash indeed, for commit logs brevity.
Closes #50.
@wI2L almost there :) I don't like to squash by force pushing - is it ok with you if you squash it yourself when merging?
Any updates @wI2L ?
@ipfans I'll take care of migrating CI to Github actions and merge pending pull requests to make a proper release by september. Sorry for the delay.
Any updates on this? I'd find it useful 😊
Merging #59 (2994b14) into master (8a048be) will decrease coverage by
0.71%
. The diff coverage is72.41%
.:exclamation: Current head 2994b14 differs from pull request most recent head c029d2c. Consider uploading reports for the commit c029d2c to get more accurate results
@@ Coverage Diff @@
## master #59 +/- ##
==========================================
- Coverage 95.88% 95.17% -0.72%
==========================================
Files 7 7
Lines 924 953 +29
==========================================
+ Hits 886 907 +21
- Misses 22 30 +8
Partials 16 16
Impacted Files | Coverage Δ | |
---|---|---|
openapi/generator.go | 94.25% <50.00%> (-0.14%) |
:arrow_down: |
openapi/spec.go | 81.57% <65.00%> (-18.43%) |
:arrow_down: |
fizz.go | 98.66% <100.00%> (+0.06%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 8a048be...c029d2c. Read the comment docs.
To everyone involved on this PR, and expecting the changes, I apologize for the unreasonable delay, haven't found some time to work on it, and I wanted to update the CI pipeline of the project before merhing anything new. This will be shipped in next release.
Thanks for getting it in @wI2L!
Fixes Issue #35, continues stale PR #50.
I tried to keep the comments from #50 relevant by resolving conflicts by back-merging and addressing them here. We can squash commits at your discretion.
security
field in path section (see security parameter - an empty array is used to signal "no security" override)The last point is the only one that is troublesome since it does not align with the way JSON gets serialised and would benefit from something like this request. The proposed way is backwards compatible but does repeat some code.