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

Introduced type keyword among the ones that need to be escaped #1066

Merged

Conversation

espinhogr
Copy link
Contributor

@espinhogr espinhogr commented Mar 3, 2019

TODO:

  • Update CHANGELOG.md* with your change (include reference to issue & this PR)
  • Make sure all of the significant new logic is covered by tests
    Tests are only there for the general case and not for each single keyword
  • Rebase your changes on master so that they can be merged easily
  • Make sure all tests and linter rules pass

Fixes: #1065

@espinhogr espinhogr force-pushed the safe_scalajs_parameter_names branch 2 times, most recently from 5d23249 to 730ee98 Compare March 3, 2019 01:56
@espinhogr
Copy link
Contributor Author

I don't know why Generated Types Check is not passing as I haven't touched the typescript generation at all.

@trevor-scheer
Copy link
Member

@espinhogr thanks for the PR! Looks like it just needs a rebase against master.

@espinhogr espinhogr force-pushed the safe_scalajs_parameter_names branch from 730ee98 to 67adb9d Compare March 4, 2019 21:21
@espinhogr
Copy link
Contributor Author

@trevor-scheer I've just rebased to the master but it seems to fail in a test on Windows, is this something known or you think it's the change I've introduced?

@trevor-scheer
Copy link
Member

@espinhogr we've got something flaky in the tests, I re-ran and it looks good. Thanks again!

@trevor-scheer trevor-scheer merged commit af17dbf into apollographql:master Mar 4, 2019
@espinhogr
Copy link
Contributor Author

@trevor-scheer Brilliant, happy to help!

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.

None yet

2 participants