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

Generate thrift with private members #3404

Open
ctubbsii opened this issue May 16, 2023 · 0 comments · May be fixed by #3405
Open

Generate thrift with private members #3404

ctubbsii opened this issue May 16, 2023 · 0 comments · May be fixed by #3405
Assignees
Labels
enhancement This issue describes a new feature, improvement, or optimization.

Comments

@ctubbsii
Copy link
Member

I made a comment on #3403 about how some of our Thrift problems where we see a default value of 0 for primitive types that aren't explicitly set, and how those are indistinguishable from the way we're setting them to 0 explicitly, because we're using the public fields to access them directly, instead of the setter/getter methods.

I believe Thrift has the ability to generate Java code with private members, which would force us to use the setters/getters, but this would be a large, tedious change. We could switch over to the setters/getters incrementally, before switching to force the code to use private members, or just switch to using private members, and force all the changes to happen to fix all the compilation issues.

To use private members, it's a simple matter of editing our script, but the tedious part is fixing all the compilation issues that would result:

--- a/core/src/main/scripts/generate-thrift.sh
+++ b/core/src/main/scripts/generate-thrift.sh
@@ -69,3 +69,3 @@ rm -rf "$BUILD_DIR"/gen-java
 for f in src/main/thrift/*.thrift; do
-  thrift "${THRIFT_ARGS[@]}" --gen java:generated_annotations=suppress "$f" || fail unable to generate java thrift classes
+  thrift "${THRIFT_ARGS[@]}" --gen java:generated_annotations=suppress,private-members "$f" || fail unable to generate java thrift classes
   thrift "${THRIFT_ARGS[@]}" --gen py "$f" || fail unable to generate python thrift classes
@ctubbsii ctubbsii added the enhancement This issue describes a new feature, improvement, or optimization. label May 16, 2023
@ctubbsii ctubbsii self-assigned this May 16, 2023
@ctubbsii ctubbsii linked a pull request May 16, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue describes a new feature, improvement, or optimization.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant