-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
PARQUET-2274: Remove Yetus #1056
Conversation
I don't think this is used anywhere anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM though I am not familiar with this tool.
cc @gszadovszky
We started to use this to mark somehow the public classes/methods that did not ment to be public for the API users but only because of our current module layout. Since we are on java8 we do not have anything else to mark them. But because we do not have any tools running on the API (not even talking about the client side) it does not really make sense to have these annotations. Maybe it would worth some comments as replacements of the annotations but I am not sure they would be checked before used. |
Yeah, adding some comments could help although sometimes people ignore it. But some of them are already used by other tool like the columnIndex one. |
@Fokko Do you think you can add some comment and we merge this PR in? |
@shangxinli Sorry for the long wait, I've added the comments 👍 |
@sekikn Do you have any comment? |
No, I agree with replacing audience annotations with appropriate comments for now, since we don't have a tool for enforcing restriction based on the annotations, as mentioned by @gszadovszky. |
Thank you @sekikn for confirming. I'll move this forward. |
I don't think this is used anywhere anymore.
Make sure you have checked all steps below.
Jira
Tests
Commits
Documentation