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

[#3081] improvement(core): Remove hack in BaseCatalog.java #3166

Merged
merged 2 commits into from
Apr 24, 2024

Conversation

noidname01
Copy link
Collaborator

@noidname01 noidname01 commented Apr 24, 2024

What changes were proposed in this pull request?

Modify the description of CATALOG_OPERATION_IMPL, avoid the use of "hack"

Why are the changes needed?

Fix: #3081

Does this PR introduce any user-facing change?

No

How was this patch tested?

No need to test

@jerryshao
Copy link
Contributor

jerryshao commented Apr 24, 2024

@noidname01 Thanks a lot for your contribution. I think the issue is misleading, this part of the code is required for the some advanced users, we just leave a back door for such users without documentation.

@noidname01
Copy link
Collaborator Author

@jerryshao Sure, so this issue is not necessary?

@jerryshao
Copy link
Contributor

I think what you need to do is to improve the comment to avoid saying that this is a hacky way. I think why @justinmclean created this issue is that he uses some tools to do code scan, that tool may trigger a warning when finding this keyword.

@noidname01
Copy link
Collaborator Author

OK, Got it

@jerryshao jerryshao added need backport Issues that need to backport to another branch branch-0.5 labels Apr 24, 2024
Copy link
Contributor

@jerryshao jerryshao left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for your contribution.

@jerryshao jerryshao merged commit 79044b2 into apache:main Apr 24, 2024
22 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 24, 2024
### What changes were proposed in this pull request?

Modify the description of `CATALOG_OPERATION_IMPL`, avoid the use of
"hack"

### Why are the changes needed?

Fix: #3081 

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?
  
No need to test

---------

Co-authored-by: TimWang <[email protected]>
diqiu50 pushed a commit to diqiu50/gravitino that referenced this pull request Jun 13, 2024
…che#3166)

### What changes were proposed in this pull request?

Modify the description of `CATALOG_OPERATION_IMPL`, avoid the use of
"hack"

### Why are the changes needed?

Fix: apache#3081 

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?
  
No need to test

---------

Co-authored-by: TimWang <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need backport Issues that need to backport to another branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Improvement] Remove this hack in BaseCatalog.java
2 participants