You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Li Pi (JIRA)" <ji...@apache.org> on 2011/08/18 22:17:29 UTC
[jira] [Created] (HBASE-4226) HFileBlock.java style cleanup.
HFileBlock.java style cleanup.
------------------------------
Key: HBASE-4226
URL: https://issues.apache.org/jira/browse/HBASE-4226
Project: HBase
Issue Type: Improvement
Reporter: Li Pi
Assignee: Li Pi
Priority: Trivial
Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13087533#comment-13087533 ]
stack commented on HBASE-4226:
------------------------------
@Li Who every told you it was a good idea to make stylistic changes to the code base? Don't you know that making style changes is like poking a hornets nest with a big old stick; even the old codger wasps are going to come out w/ stingers deployed.
In below change:
{code}
- throw new IOException("Block type stored in the buffer: " +
- blockTypeFromBuf + ", block type field: " + blockType);
+ throw new IOException("Block type stored in the buffer: "
+ + blockTypeFromBuf + ", block type field: " + blockType);
{code}
... i personally prefer the former where there is a hanging '+' on the end of the line. The hanging plus indicates a line continued (w/o the '+' I find you need to do a bit more work to figure next line is a continuation).
Here, stylisitically, I like that the second parameter is complete on the second line rather than broken across lines:
{code}
- Preconditions.checkState(state != State.INIT,
- "Unexpected state: " + state);
+ Preconditions.checkState(state != State.INIT, "Unexpected state: "
+ + state);
{code}
So, I agree w/ about 2/3rds of this patch.
(My guess is that if you fix the above so I like it, Ted Yu is going to show up next and disagree w/ a different third of the changes -- smile)
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff, hbase-4226v2.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "Li Pi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Li Pi updated HBASE-4226:
-------------------------
Attachment: hbase-4226.diff
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "Li Pi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13087337#comment-13087337 ]
Li Pi commented on HBASE-4226:
------------------------------
Gotcha. Will fix.
https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13087298#comment-13087298]
places where we have incorrect style (eg braces on the wrong line) but I
don't think it's worth rewrapping existing code that fits the style
guidelines. (eg a lot of the javadoc changes are just re-justifying)
prefetchedHeader.buf
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "Todd Lipcon (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13087298#comment-13087298 ]
Todd Lipcon commented on HBASE-4226:
------------------------------------
A lot of these changes actually make it harder to read. I'm all for fixing places where we have incorrect style (eg braces on the wrong line) but I don't think it's worth rewrapping existing code that fits the style guidelines. (eg a lot of the javadoc changes are just re-justifying)
some examples:
- if (uncompressedSizeWithoutHeader !=
- uncompressedBytesWithHeader.length - HEADER_SIZE) {
+ if (uncompressedSizeWithoutHeader != uncompressedBytesWithHeader.length
+ - HEADER_SIZE) {
(harder to read after)
- InputStream bufferedBoundedStream = createBufferedBoundedStream(
- offset, onDiskSize, pread);
+ InputStream bufferedBoundedStream = createBufferedBoundedStream(offset,
+ onDiskSize, pread);
IMO I think the original was better
- ByteBuffer headerBuf = prefetchedHeader.offset == offset ?
- prefetchedHeader.buf : null;
+ ByteBuffer headerBuf = prefetchedHeader.offset == offset ? prefetchedHeader.buf
+ : null;
same
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "Li Pi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13087539#comment-13087539 ]
Li Pi commented on HBASE-4226:
------------------------------
I'm gonna cut this patch down to the single bit about brackets. I think
that, we can all agree, is bad. Or I might just sneak fixes into 4027.
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff, hbase-4226v2.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13087570#comment-13087570 ]
stack commented on HBASE-4226:
------------------------------
The above sounds smart Li (especially the sneaking stuff in -- smile)
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff, hbase-4226v2.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "Li Pi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Li Pi updated HBASE-4226:
-------------------------
Attachment: hbase-4226v2.diff
far more conservative cleanup.
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff, hbase-4226v2.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "Li Pi (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Li Pi updated HBASE-4226:
-------------------------
Status: Patch Available (was: Open)
just a simple code cleanup.
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HBASE-4226) HFileBlock.java style cleanup.
Posted by "Li Pi (Updated) (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-4226?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Li Pi updated HBASE-4226:
-------------------------
Resolution: Not A Problem
Status: Resolved (was: Patch Available)
> HFileBlock.java style cleanup.
> ------------------------------
>
> Key: HBASE-4226
> URL: https://issues.apache.org/jira/browse/HBASE-4226
> Project: HBase
> Issue Type: Improvement
> Reporter: Li Pi
> Assignee: Li Pi
> Priority: Trivial
> Attachments: hbase-4226.diff, hbase-4226v2.diff
>
>
> Just a simple style cleanup of HFileBlock.java.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira