-
-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
render empty oauth scopes #290
render empty oauth scopes #290
Conversation
@@ -36,6 +36,9 @@ type findValueUntyped interface { | |||
// ToYamlNode converts the ordered map to a yaml node ready for marshalling. | |||
func (o *Map[K, V]) ToYamlNode(n NodeBuilder, l any) *yaml.Node { | |||
p := utils.CreateEmptyMapNode() | |||
if o != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a small test for this, as this new block will reduced coverage, if you pull the project again and update the workflow config (upgraded the codecov action) You should see the coverage on this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merged the latest change from main branch and ran go test -v -coverprofile cover.out ./...
locally and see 100% coverage. In the status on this PR I see 1 workflow awaiting approval
, which I assume means you need to approve something before the coverage check can run here, but I'm unfamiliar. I'd appreciate if you clarify for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whoops, nevermind I was reading the test output incorrectly. I'll try to figure it out.
@daveshanley One edge case I noticed when testing: I had:
And with these changes in libopenapi it is now being processed and spit out as
It doesn't matter for this case since using |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #290 +/- ##
=======================================
Coverage 99.62% 99.62%
=======================================
Files 162 162
Lines 16075 16077 +2
=======================================
+ Hits 16014 16016 +2
Misses 56 56
Partials 5 5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I have no idea where Because I have no idea, let's just proceed, if someone else finds this issue then we can investigate further. |
see #289
This is my naive attempt to fix the issue. It works but it may have negative side-effects I'm not aware of.