Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
wgtmac commented on PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#issuecomment-1833367177 > I rebased on the latest master. I'd rather get this merged somewhat quickly (maybe by the end of the week?) as to avoid blocking other merges and/or endless rebasing. I just merged this PR to unblock others. Thanks @amousavigourabi! -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
wgtmac merged PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209 -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
Fokko commented on PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#issuecomment-1832643602 @amousavigourabi I can see that it is not nice having to rebase all the time. I can hold off on mine, they are probably easier to resolve than yours. -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
amousavigourabi commented on PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#issuecomment-1832537608 @Fokko , how do we want to coordinate this pull request and your refactoring PRs? I do not feel a lot for applying the editorconfig and Spotless a dozen or so times to resolve conflicts in this PR after each of those get merged. -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
amousavigourabi commented on code in PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#discussion_r1409749808 ## pom.xml: ## @@ -512,6 +515,43 @@ + + +com.diffplug.spotless +spotless-maven-plugin +${spotless.version} + + + + **/generated-sources/** + + + + + + + + + + true + 4 + Review Comment: It is a bit hacky, but it also allows contributors to run the `spotless:apply` goal in case they are for whatever reason using tabs in their development environment. -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
amousavigourabi commented on code in PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#discussion_r1409748314 ## pom.xml: ## @@ -512,6 +515,43 @@ + + +com.diffplug.spotless +spotless-maven-plugin +${spotless.version} + + + + **/generated-sources/** + + + + + + + + + + true + 4 + Review Comment: No, tabs are _not_ allowed by this configuration. This block is a way to make the linter work as it should with two space indents as used by Parquet, instead of four. -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
Fokko commented on PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#issuecomment-1831861024 This is great @amousavigourabi. I was thinking of adding linting as well, so great work here! -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
gszadovszky commented on code in PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#discussion_r1408884415 ## pom.xml: ## @@ -512,6 +515,43 @@ + + +com.diffplug.spotless +spotless-maven-plugin +${spotless.version} + + + + **/generated-sources/** + + + + + + + + + + true + 4 + Review Comment: Does it mean we allow tabs for indention? (Personally, I don't like tabs because it might be rendered differently on different systems.) -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
wgtmac commented on PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#issuecomment-1831045581 Thanks for the improvement! Could you please take a look at this? @shangxinli @gszadovszky @Fokko -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
amousavigourabi commented on PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#issuecomment-1830328306 The `.editorconfig` has been expanded for IntelliJ and is mostly compliant with the Spotless configuration. IntelliJ refactoring and Spotless have some minor disagreements on continuation indents sometimes, which cannot really be resolved at the moment. As it is included in the Maven lifecycle, the Spotless configuration would of course be leading. -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]
amousavigourabi commented on PR #1209: URL: https://github.com/apache/parquet-mr/pull/1209#issuecomment-1830320974 @wgtmac -- 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: dev-unsubscr...@parquet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org