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

Address review comments on PR #24539 #24553

Closed
bzbarsky-apple opened this issue Jan 20, 2023 · 1 comment · Fixed by #24555
Closed

Address review comments on PR #24539 #24553

bzbarsky-apple opened this issue Jan 20, 2023 · 1 comment · Fixed by #24555

Comments

@bzbarsky-apple
Copy link
Contributor

  1. Instead of codegen-cpp-only do something like --generator codegen --codegen cpp-app
  2. Instead of messing with the filter, just avoid adding CodegenJavaPregenerator to the list if codegen is cpp-app.
@andy31415
Copy link
Contributor

We may also just want to extend the --generator format to standardize on some separator. Like accept codegen but also codegen:java or codegen:cpp-app to control the internal generator list.

In the PR review I am basically asking to not special case cpp-app but make it more generic (and also control generator array instead of passing down this to accept methods since the intent seems to control the generator)

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 a pull request may close this issue.

2 participants