path unescape url when resolving remote reference #142
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #142 +/- ##
=======================================
Coverage 99.79% 99.79%
=======================================
Files 146 146
Lines 10349 10373 +24
=======================================
+ Hits 10328 10352 +24
Misses 21 21
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
| @@ -189,6 +189,11 @@ func (index *SpecIndex) lookupRemoteReference(ref string) (*yaml.Node, *yaml.Nod | |||
| } else { | |||
| query = "$" | |||
| } | |||
|
|
|||
| query, err := url.PathUnescape(query) | |||
There was a problem hiding this comment.
Nice upgrade!
My only comment is that this branch will drop code coverage without a test to check for the invalid case.
There was a problem hiding this comment.
thank you. Sure I am going to add some testing
| @@ -284,6 +289,11 @@ func (index *SpecIndex) lookupFileReference(ref string) (*yaml.Node, *yaml.Node, | |||
| query = "$" | |||
| } | |||
|
|
|||
| query, err := url.PathUnescape(query) | |||
| if err != nil { | |||
There was a problem hiding this comment.
Same comment as above about the coverage.
|
Did you decide to stop working on this? |
|
Hi, Sorry something went wrong when I force pushed. Github automatically closed this when it saw that the main branch has no additional commits compared to the upstream main branch. Working on it. |
|
@daveshanley FYI, I have added some unit tests. |
|
Thanks, it seems like codecove server is encoutring some issues: Would you mind relaunching the build please ? |
|
I re-ran the build, there is just one single line missing from the coverage (that's causing it to be red) https://app.codecov.io/gh/pb33f/libopenapi/pull/142/blob/index/find_component.go#L294 |
|
thanks. I added an additional test to cover the missing line |
|
thanks @daveshanley |
|
@daveshanley, are you planning on creating a tag/release for this patch ? |
|
It was released in |
|
great thanks! |
closes #141