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.

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

[PHP] Fix username and password check #1892

Merged

Conversation

jfastnacht
Copy link
Contributor

@jfastnacht jfastnacht commented Jan 11, 2019

PR checklist

  • 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 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\.
  • 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.

Description of the PR

Trying to fix the issue #1408 . Intermediate result, WIP.

Had some problems with the test under Windows 8.1, so it might need a little more than the usual review.

@jebentier @dkarlovi @mandrean @jfastnacht @ackintosh @ybelenko @renepardon

@auto-labeler auto-labeler bot added the WIP Work in Progress label Jan 11, 2019
@auto-labeler
Copy link

auto-labeler bot commented Jan 11, 2019

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@jfastnacht
Copy link
Contributor Author

Sometimes the builds are breaking on incomprehensible, random points. Things should be fixed, but this PR goes further than only #1408.
There is a lot of File.separator usage in the PHP projects where it is unnecessary. In PHP you should always use /, even on Windows. There are probably more of these left in other PHP generator files.

I also had to apply the fix in #1863 to make the build work on WIndows, I haven't pushed these changes, since they are already part of the master-branch.

The original issue should be solved now, but I'm not sure if it would be a better idea to split up this PR into seperate issues to discuss and PRs to pull.

@wing328
Copy link
Member

wing328 commented Jan 30, 2019

Sometimes the builds are breaking on incomprehensible, random points. Things should be fixed, but this PR goes further than only #1408.

Yup, especially weekend as we saw issues downloading dependencies from time to time. Restarting the job (close and reopen the PR) usually fixes the problem

@jfastnacht
Copy link
Contributor Author

Restarting the job (close and reopen the PR) usually fixes the problem

I didn't knew that you can restart it this way, thanks.

@jfastnacht jfastnacht closed this Jan 30, 2019
@jfastnacht jfastnacht reopened this Jan 30, 2019
@jfastnacht jfastnacht closed this Feb 15, 2019
@jfastnacht jfastnacht reopened this Feb 15, 2019
@wing328
Copy link
Member

wing328 commented Feb 18, 2019

@jfastnacht is this PR ready for review? If yes, I'll remove WIP from the label and the title.

@jfastnacht jfastnacht changed the title [WIP][PHP] Username checks #1408 [PHP] Username checks #1408 Feb 18, 2019
@jfastnacht
Copy link
Contributor Author

@wing328 tbh, this PR is a mess, because it changes much more than the initial issue states. Not sure if this would be okay anyways, otherwise I'll break it up into seperate issues and PRs.

@wing328
Copy link
Member

wing328 commented Mar 7, 2019

@jfastnacht I'd a look and I think it's acceptable as your fix is essentially just one line: https://github.com/OpenAPITools/openapi-generator/pull/1892/files#diff-076573ccfdac2ab5af4dc1813e5ccee3R584

@wing328 wing328 removed the WIP Work in Progress label Mar 7, 2019
@wing328 wing328 added this to the 4.0.0 milestone Mar 7, 2019
@wing328 wing328 merged commit c00a439 into OpenAPITools:master Mar 7, 2019
@wing328 wing328 changed the title [PHP] Username checks #1408 [PHP] Fix username and password check Mar 7, 2019
jimschubert added a commit to jimschubert/openapi-generator that referenced this pull request Mar 9, 2019
* master: (758 commits)
  Add support for free form requests (OpenAPITools#2288)
  [typescript-rxjs] drop unneeded function wrapping  (OpenAPITools#2332)
  [typescript-fetch] Guard array mapping against undefined on optional array model properties (OpenAPITools#2324)
  Fix regex in Python server model code (OpenAPITools#2314)
  Add .travis.yml and Gemfile.lock to ruby security test folder (OpenAPITools#2330)
  Add a link to CSDN article (OpenAPITools#2331)
  [Maven] fix Spaces in Windows user path breaks build on test goal (OpenAPITools#2318)
  [PHP] fix bad links in Model docs (OpenAPITools#2316)
  [java]: fix datatype for non-multipart file request body (OpenAPITools#2271)
  Removed JFCote from core team (OpenAPITools#2315)
  [R sample] fix CircleCI error of outdated sample (OpenAPITools#2313)
  [Java] Bean Validation for decimalmin/max incorrect when exclusive set (OpenAPITools#2115)
  Java Spring : fix defaultValue annotation double quoted in api operation (OpenAPITools#2267)
  Java RESTEASY : fix defaultValue annotation double quoted in api operation (OpenAPITools#2268)
  [PHP] Username checks OpenAPITools#1408 (OpenAPITools#1892)
  [typescript-fetch] remove namespaces in enums (OpenAPITools#2123)
  [java-server-msf4j] fix and upgrade (OpenAPITools#2303)
  fix test script path in CONTRIBUTING.md (OpenAPITools#2290)
  Dart queryargs (OpenAPITools#2250)
  add Blueplanet language (OpenAPITools#2184)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants