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

C sharp reserved keywords #4250

Merged
merged 1 commit into from
Jun 22, 2020
Merged

C sharp reserved keywords #4250

merged 1 commit into from
Jun 22, 2020

Conversation

cklam2
Copy link
Contributor

@cklam2 cklam2 commented Jun 17, 2020

Attempt to solve the issue with reserved keywords, see #4248. Names will now be prefixed with @ when it is a keyword.

Note that the list of keywords is static. If Microsoft decides to extend the list then we'd have to manually update the list.

Note that changes are based on the c-sharp-improvements branch which is not yet merged to master. Hopefully that is not a big issue.

@dotansimha
Copy link
Owner

@cklam2 can you please rebase? :)
Thanks!

@cklam2
Copy link
Contributor Author

cklam2 commented Jun 18, 2020

@dotansimha thanks. I will first make a change in the code. Want to make use of that convertName feature instead. Will mention you again when rebased and ready for review/merge.

Verified

This commit was signed with the committer’s verified signature.
albertvillanova Albert Villanova del Moral
@cklam2
Copy link
Contributor Author

cklam2 commented Jun 20, 2020

Updated the code and rebased onto master. Also took the chance to clean up the unit test so format looks cleaner and bit more in line with other unit tests. Btw, new unit tests for this bug are also added.
@dotansimha code is ready for review/merge, thanks.

@dotansimha dotansimha merged commit fd99e06 into dotansimha:master Jun 22, 2020
dotansimha added a commit that referenced this pull request Jun 22, 2020

Verified

This commit was signed with the committer’s verified signature.
albertvillanova Albert Villanova del Moral
…#4248) (#4250)"

This reverts commit fd99e06.
@dotansimha
Copy link
Owner

@cklam2 I merged this one, it includes all changes from #4264 as well, right?

@theguild-bot
Copy link
Collaborator

The latest changes of this PR are available as alpha in npm: 1.15.5-alpha-fd99e06a.0

Quickly update your package.json by running:

npx match-version @graphql-codegen 1.15.5-alpha-fd99e06a.0

@cklam2
Copy link
Contributor Author

cklam2 commented Jun 22, 2020

@dotansimha thanks for the merge!
#4264 is not included in this one. I rebased that branch onto the new master.

Btw, I see a branch "revert-4250-c-sharp-reserved-keywords"?

@dotansimha
Copy link
Owner

@cklam2 thanks! The revert branch was a mistake :)
I merged #4264 as well. Thank you so much !

@cklam2 cklam2 deleted the c-sharp-reserved-keywords branch June 23, 2020 15:35
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

3 participants