Skip to content

Conversation

SlavchoArsovski
Copy link

PR checklist

  • [ DONE] Read the contribution guidelines.
  • [ DONE] Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • [DONE ] Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.
  • [ DONE] Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

Fix for:
File path for the specification was not sanitized and failing for Windows.

@SlavchoArsovski
Copy link
Author

@HugoMario could you review? this is fix for the blocker issues:
#10844
#10823

@SlavchoArsovski SlavchoArsovski changed the title sanitize specification, so it works for all types of OS. Fix for Wind… sanitize specification url, so it works for all types of OS. Fix for Wind… Feb 2, 2021
@HugoMario
Copy link
Contributor

thanks @SlavchoArsovski ,

Going to check (and merge) this after next release.

@SlavchoArsovski
Copy link
Author

SlavchoArsovski commented Feb 9, 2021

@HugoMario, don't you think that would be too much time to wait for a urgent and small fix?

@SlavchoArsovski
Copy link
Author

@HugoMario is there a chance to merge it, sry for being persistent but the branch is opened more than a month :)

@HugoMario
Copy link
Contributor

Hi @SlavchoArsovski sorry for delay

Yea, this PR LGTM but i need to test your changes first make sure it works in differents OS as the title suggest. Just a doubt, why to create a one line method to this, instead just to call the string method directly?

@SlavchoArsovski
Copy link
Author

Hi @SlavchoArsovski sorry for delay

Yea, this PR LGTM but i need to test your changes first make sure it works in differents OS as the title suggest. Just a doubt, why to create a one line method to this, instead just to call the string method directly?

From my side, it was tested on Windows 10 and Macintosh V10.15.7. Feel free to test it.
I have created a new line method as the expression is now used on two places.

@HugoMario HugoMario merged commit 4f4a7af into swagger-api:3.0.0 Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants