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

feat(java): Add 'Expose' annotation to support "only de/serialize annotated fields" #1751

Merged
merged 5 commits into from
Jul 23, 2024

Conversation

urlyy
Copy link
Contributor

@urlyy urlyy commented Jul 22, 2024

What does this PR do?

Add Expose annotation to support "only de/serialize annotated fields"

Related issues

#1735

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

N/A

@urlyy urlyy requested a review from chaokunyang as a code owner July 22, 2024 06:44
@urlyy
Copy link
Contributor Author

urlyy commented Jul 22, 2024

@chaokunyang I have already modified the code style according to the output of mvn checkstyle:check, but it still did not pass the GitHub Actions check. Can you help me identify the reason?

@chaokunyang
Copy link
Collaborator

image

You need to format code by mvn spotless:apply

@chaokunyang
Copy link
Collaborator

The code looks good, but I have concerns about the annotation name. This is the hard one. We have many choices such as Expose/SerializeField/FuryField/Include/FuryInclude. I'm not sure which is better

@chaokunyang
Copy link
Collaborator

chaokunyang commented Jul 23, 2024

I think we can name it as Expose, and in future we can add more advanced annotation like:

@interface Include {
  String[] fieldsNames();
}

@interface Exclude {
  String[] fieldsNames();
}

@Include(fieldsNames={"f1"})
class Foo {
  inf f1;
  bool f2;
}

@Exclude(fieldsNames={"f2"})
class Foo {
  inf f1;
  bool f2;
}

@LiangliangSui do you have any suggestions?

@urlyy urlyy changed the title feat(java): Add 'Exposed' annotation to support "only de/serialize annotated fields" feat(java): Add 'Expose' annotation to support "only de/serialize annotated fields" Jul 23, 2024
Copy link
Collaborator

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chaokunyang chaokunyang merged commit 895c6e1 into apache:main Jul 23, 2024
34 checks passed
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.

2 participants