Re: [PR] PARQUET-2386: More consistent code style in parquet-mr [parquet-mr]

2023-11-30 Thread via GitHub


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]

2023-11-30 Thread via GitHub


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]

2023-11-29 Thread via GitHub


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]

2023-11-29 Thread via GitHub


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]

2023-11-29 Thread via GitHub


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]

2023-11-29 Thread via GitHub


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]

2023-11-29 Thread via GitHub


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]

2023-11-29 Thread via GitHub


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]

2023-11-28 Thread via GitHub


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]

2023-11-28 Thread via GitHub


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]

2023-11-28 Thread via GitHub


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