-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-25465 Use javac --release option for supporting cross version c… #4164
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
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
The result is overall good. Let me reply on the mailing list. |
apurtell
left a comment
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.
This is good work. I left a couple of comments where I think it didn't go quite far enough and more concerns should be moved out -- UnsafeAccess and ClassSize. Could be done as follow up work.
| this.unsafeRef = buf.array(); | ||
| } else { | ||
| this.unsafeOffset = ((DirectBuffer) buf).address(); | ||
| this.unsafeOffset = UnsafeAccess.directBufferAddress(buf); |
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.
Should we move all of UnsafeAccess into HBasePlatformDependent?
| int headerSize() { | ||
| try { | ||
| return (int) UnsafeAccess.theUnsafe.objectFieldOffset( | ||
| return (int) HBasePlatformDependent.objectFieldOffset( |
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.
This is platform dependent so should be moved into HBasePlatformDependent. Perhaps all of ClassSize should go up in there but at least these methods involving object layout concerns can be replaced with external static helpers in the thirdparty module.
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…ompilation (#4164) Signed-off-by: Andrew Purtell <[email protected]>
…ompilation (#4164) Signed-off-by: Andrew Purtell <[email protected]>
…ompilation (apache#4164) Signed-off-by: Andrew Purtell <[email protected]>
* HBASE-26523 Upgrade hbase-thirdparty dependency to 4.0.1 (#3988) Signed-off-by: GeorryHuang <[email protected]> * HBASE-25465 Use javac --release option for supporting cross version compilation (#4164) Signed-off-by: Andrew Purtell <[email protected]> * HBASE-26855 Delete unnecessary dependency on jaxb-runtime jar (#4236) Signed-off-by: Duo Zhang <[email protected]> * spotless apply Co-authored-by: Duo Zhang <[email protected]> Co-authored-by: Nick Dimiduk <[email protected]> Signed-off-by: Andrew Purtell <[email protected]>
…e#4439) * HBASE-26523 Upgrade hbase-thirdparty dependency to 4.0.1 (apache#3988) Signed-off-by: GeorryHuang <[email protected]> * HBASE-25465 Use javac --release option for supporting cross version compilation (apache#4164) Signed-off-by: Andrew Purtell <[email protected]> * HBASE-26855 Delete unnecessary dependency on jaxb-runtime jar (apache#4236) Signed-off-by: Duo Zhang <[email protected]> * spotless apply Co-authored-by: Duo Zhang <[email protected]> Co-authored-by: Nick Dimiduk <[email protected]> Signed-off-by: Andrew Purtell <[email protected]> (cherry picked from commit 91a44f5) Change-Id: I32d8a4fc4f7da7e24ce82d1d90cc9f22b4238599
…ompilation