Skip to content

Conversation

@ptitjes
Copy link

@ptitjes ptitjes commented Feb 6, 2021

This PR fixes a code gen problem introduced in #6420. The generated code for "endXXStruct()" doesn't compile. I assume that there was a regexp used to replace .required by .IsRequired. It was also applied incorrectly to builder.requiredField.

To test it I did:

make
npm install
npm run test

It seem the make run did modifications to some generated .h files that are totally unrelated to my fix. I suspect that these were not correctly regenerated by a previous PR. I included those changes as a separate commit. Tell me if you want to drop this commit.

Also when I do cd test, sh generate_code.sh the changes on the .h are reverted and a whole lot of new .js|.ts|.d.ts files are created at the root of the tests directory and not in the tests/ts directory (which already contains the same files). Is that supposed to happen ? Please tell me what to do.

@google-cla
Copy link

google-cla bot commented Feb 6, 2021

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@github-actions github-actions bot added c++ codegen Involving generating code from schema javascript typescript labels Feb 6, 2021
@ptitjes ptitjes force-pushed the fix-gen-ts-after-6420 branch from bc3ffcf to cf782ed Compare February 6, 2021 13:23
@ptitjes
Copy link
Author

ptitjes commented Feb 6, 2021

@googlebot I fixed it.

@ptitjes
Copy link
Author

ptitjes commented Feb 6, 2021

@krojew Would you mind helping me make my two PRs pass the CIs ? I think the problems come from the generated files.

@aardappel
Copy link
Collaborator

@bjornharrtell

@krojew
Copy link
Contributor

krojew commented Mar 10, 2021

@ptitjes did you run tests/generate_code.sh after making changes?

@ptitjes
Copy link
Author

ptitjes commented Mar 11, 2021

@ptitjes did you run tests/generate_code.sh after making changes?

Hey @krojew! I tried but as I said in the description, it also generated lots of unrelated changes.

I will rebase and try again this week-end.

@bjornharrtell
Copy link
Collaborator

This is already fixed on master with #6495?

@ptitjes
Copy link
Author

ptitjes commented Apr 3, 2021

This is already fixed on master with #6495?

Hmm, yeah it seems @krojew redid the same fix. :( Closing.

@ptitjes ptitjes closed this Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ codegen Involving generating code from schema javascript typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants