Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Ruby] Fix regualr expression in error message #2139

Merged
merged 2 commits into from
Feb 18, 2019
Merged

[Ruby] Fix regualr expression in error message #2139

merged 2 commits into from
Feb 18, 2019

Conversation

autopp
Copy link
Contributor

@autopp autopp commented Feb 13, 2019

  • Read the contribution guidelines.
  • 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, ./bin/openapi3/{LANG}-petstore.sh, ./bin/security/{LANG}-petstore.sh and ./bin/openapi3/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\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Fix #2069.

  • To correctly embedding the regualr expression into error message, it's changed to string interpolation.
  • Unnecessary invocation of Regexp.new is deleted. (Regexp.new(/.../) -> /.../)

@wing328
Copy link
Member

wing328 commented Feb 13, 2019

@autopp thanks for the PR. If my memory serves me well, the reason of using Regexp.new is that not all pattern specified in the spec follows the /.../ pattern.

@autopp
Copy link
Contributor Author

autopp commented Feb 13, 2019

@wing328 Thanks for review!

If my memory serves me well, the reason of using Regexp.new is that not all pattern specified in the spec follows the /.../ pattern.

I thought that all patterns follow the /.../ because all patterns are processed by toRegularExpression and addRegularExpressionDelimiter.

In any case, since this is not directly related to the purpose of this PR, I will remove it once.
If it really is OK to remove this, I will re-submit Issue and PR.

@wing328
Copy link
Member

wing328 commented Feb 13, 2019

@autopp let me try to dig out the discussion by this weekend. Then we can decide whether to proceed with this PR or not. Stay tuned.

@autopp autopp force-pushed the fix-pattern-escaping-in-ruby-client branch from 54e78fe to 7bafde9 Compare February 13, 2019 14:41
@autopp
Copy link
Contributor Author

autopp commented Feb 13, 2019

@wing328 I rebased.

@autopp
Copy link
Contributor Author

autopp commented Feb 13, 2019

@wing328 Oh, I'm sorry...I fixed it before I saw your reply 😢
We can set it back again if necessary, so leave it until the weekend.

@wing328
Copy link
Member

wing328 commented Feb 13, 2019

@autopp that's ok bro.

@ackintosh
Copy link
Contributor

https://travis-ci.org/OpenAPITools/openapi-generator/builds/492733373?utm_source=github_status&utm_medium=notification

The job exceeded the maximum time limit for jobs, and has been terminated.

I've restarted the job. 😉

@autopp
Copy link
Contributor Author

autopp commented Feb 14, 2019

@ackintosh Thanks!
Looking at the recent Travis's build history, it seems that there are many things that have exceeded the time limit.
I think it would be nice to parallelize the job of Travis with Build stage, for example.
https://docs.travis-ci.com/user/build-stages/
(Because I am not familiar with CI jobs, this may be misplaced.)

@autopp
Copy link
Contributor Author

autopp commented Feb 18, 2019

@wing328 How shall we do this PR after all?
If it takes time to check the necessity of Regexp.new, I think that it is possible to merge in this state once.

@wing328
Copy link
Member

wing328 commented Feb 18, 2019

@autopp let's go with what you've at the moment. We can always update the templates to handle more edge cases.

@wing328 wing328 merged commit 8d6278b into OpenAPITools:master Feb 18, 2019
@wing328 wing328 added this to the 4.0.0 milestone Feb 18, 2019
@autopp autopp deleted the fix-pattern-escaping-in-ruby-client branch February 18, 2019 14:00
@autopp
Copy link
Contributor Author

autopp commented Feb 18, 2019

@wing328 Thanks!

jimschubert added a commit that referenced this pull request Feb 23, 2019
@jimschubert
Merge branch 'master' into csharp-refactor-improvements
b38e317 
* master: (40 commits)
  [Python] remove default value from being fallback to example (#2213)
  Add petstore integration tests to Ruby OAS3 client (#2211)
  Gradle - make GenerateTask properties optional (#2185)
  skip bats installation (#2198)
  Something in the dependencies changed. This switch is no longer needed. (#1850)
  Use oauth token for basic bearer auth in Rust. (#2161)
  Fix missing nullable (#2189)
  Enable error handling in Java WebClient library, fixes #1243 (#1244)
  [core] fix referenced enum case (#2175)
  rest-template: allow array parameters in path using collectionFormat (#2177)
  update go petstore samples
  Fix string types for cpprestsdk client generator (#1676)
  update kotline samples
  Remove API Key Authentication code for go when cookie is used. (#1601)
  changed the package install instructions to install the .tgz package … (#1989)
  okhttp-gson: allow array parameters in path using collectionFormat (#2137)
  [Ruby] Fix regualr expression in error message (#2069) (#2139)
  [kotlin][client] bytearray conversion (#2166)
  [rust-server] Added client documentation to rust-server (#2159)
  [Java] Getter/Setter naming convention not followed in generated models (#2095)
  ...
A-Joshi pushed a commit to ihsmarkitoss/openapi-generator that referenced this pull request Feb 27, 2019
@autopp @wing328
[Ruby] Fix regualr expression in error message (OpenAPITools#2069) (O…
846c8fd 
…penAPITools#2139)

* Fix usage of regular expression literals in Ruby client (OpenAPITools#2069)

* update samples of Ruby client (OpenAPITools#2069)
@wing328 wing328 changed the title [Ruby] Fix regualr expression in error message (#2069) [Ruby] Fix regualr expression in error message Apr 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Client: Ruby Issue: Bug
Projects
None yet
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@autopp @wing328 @ackintosh