JFinis commented on code in PR #217:
URL: https://github.com/apache/parquet-format/pull/217#discussion_r1347243194


##########
Encodings.md:
##########
@@ -32,7 +32,7 @@ intended to be the simplest encoding.  Values are encoded 
back to back.
 
 The plain encoding is used whenever a more efficient encoding can not be used. 
It
 stores the data in the following format:
- - BOOLEAN: [Bit Packed](#BITPACKED), LSB first
+ - BOOLEAN: [Bit Packed](#BITPACKED)

Review Comment:
   IMHO, LSB first is pretty understandable and leaving it out makes the spec 
ambiguous. 
   
   Maybe it's only how I read it, but for me, LSB first pretty clearly conveys 
that "the first value will be stored in the least significant bit", so If I 
have the byte `0b00000001`, this byte encodes 8 values, with the **first** one 
being 1 and all others being 0.
   
   Maybe it helps spelling out ot more clearly, e.g.,
   
   > LSB first, i.e., the first value is stored in the least significant bit.
   
   but for someone writing bit fiddling logic - which someone writing an 
encoder/decoder has to do - `LSB first` should be descriptive enough on its own 
IMHO.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to