alamb commented on code in PR #576: URL: https://github.com/apache/parquet-format/pull/576#discussion_r3363315915
########## Compression.md: ########## @@ -48,7 +48,7 @@ No-op codec. Data is left uncompressed. A codec based on the [Snappy compression format](https://github.com/google/snappy/blob/master/format_description.txt). If any ambiguity arises when implementing this format, the implementation -provided by Google Snappy [library](https://github.com/google/snappy/) +provided by the [Snappy compression library](https://github.com/google/snappy/) Review Comment: @emkornfield noted on the original PR -- what is the rationale or removing the Google credit? ########## README.md: ########## @@ -278,7 +278,9 @@ Changes to this core format definition are proposed and discussed in depth on th ## Code of Conduct -We hold ourselves and the Parquet developer community to a code of conduct as described by [Twitter OSS](https://engineering.twitter.com/opensource): <https://github.com/twitter/code-of-conduct/blob/master/code-of-conduct.md>. +We hold ourselves and the Parquet developer community to two codes of conduct: +1. [The Apache Software Foundation Code of Conduct](https://www.apache.org/foundation/policies/conduct.html) +2. [The Apache Software Foundation Code of Conduct for GitHub](https://github.com/apache/.github/blob/main/CODE_OF_CONDUCT.md) Review Comment: This link doesn't seem to go anywhere: https://github.com/apache/.github/blob/main/CODE_OF_CONDUCT.md <img width="1179" height="698" alt="Image" src="https://github.com/user-attachments/assets/d93ce336-cc80-4979-a81b-933f1ae0d5bc"; /> I think the The Apache Software Foundation Code of Conduct is a good enough link ########## Compression.md: ########## @@ -89,7 +89,7 @@ switch to the newer, interoperable `LZ4_RAW` codec. ### ZSTD A codec based on the Zstandard format defined by -[RFC 8478](https://tools.ietf.org/html/rfc8478). If any ambiguity arises +[RFC 8878](https://tools.ietf.org/html/rfc8878). If any ambiguity arises Review Comment: I was curious so I double checked: It seems like https://datatracker.ietf.org/doc/html/rfc8478 is the older version and https://tools.ietf.org/html/rfc8878 is a newer version of the same spec The changes are listed here: https://datatracker.ietf.org/doc/html/rfc8878#name-changes-since-rfc-8478 I am not sure it really makes a difference to be honest, but this seems to be ok ########## README.md: ########## @@ -190,11 +190,11 @@ In order we have: The value of `uncompressed_page_size` specified in the header is for all the 3 pieces combined. -The encoded values for the data page is always required. The definition and repetition levels +The encoded values for the data page are always required. The definition and repetition levels are optional, based on the schema definition. If the column is not nested (i.e. -the path to the column has length 1), we do not encode the repetition levels (it would -always have the value 1). For data that is required, the definition levels are -skipped (if encoded, it will always have the value of the max definition level). +the path to the column has length 1), we do not encode the repetition levels (they would +always have the value 0). For data that is required, the definition levels are Review Comment: is this change from 1 to 0 correct? -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
