You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/06/29 08:44:40 UTC

[GitHub] [spark] gaborgsomogyi opened a new pull request #28945: [MINOR][SQL]Fix spaces in JDBC connection providers

gaborgsomogyi opened a new pull request #28945:
URL: https://github.com/apache/spark/pull/28945


   ### What changes were proposed in this pull request?
   JDBC connection providers implementation formatted in a wrong way. In this PR I've fixed the formatting.
   
   ### Why are the changes needed?
   Wrong spacing in JDBC connection providers.
   
   ### Does this PR introduce _any_ user-facing change?
   No.
   
   ### How was this patch tested?
   Existing unit tests.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] srowen commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651140854


   I don't know if we even have a strong convention for this, but I slightly prefer the double-indent for 'extends', to separate it from the body. Doesn't matter. If it's not consistent we might just leave it as is across the code.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651144009


   @dongjoon-hyun showed this: https://github.com/databricks/scala-style-guide/blame/master/README.md#L242


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651899622


   **[Test build #124653 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124653/testReport)** for PR 28945 at commit [`e5f3562`](https://github.com/apache/spark/commit/e5f356263f757ee02454f6d1a8db52244a7a46d9).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28945: [MINOR][SQL]Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651025208






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651104280


   Previously I've received suggestions not to pollute original PR intention w/ side effects like this. I'm basically fine w/ either way (but my vote goes definitely to your suggestion not to waste jenkins resources).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651178656


   **[Test build #124642 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124642/testReport)** for PR 28945 at commit [`e5f3562`](https://github.com/apache/spark/commit/e5f356263f757ee02454f6d1a8db52244a7a46d9).


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28945: [MINOR][SQL]Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651022589


   cc @dongjoon-hyun @maropu 
   I thought it would be overkill to create a jira for this but if you think just let me know.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651025208






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-652087914


   Test PASSed.
   Refer to this link for build results (access rights to CI server needed): 
   https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/124653/
   Test PASSed.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651094453


   It's actually not encouraged to make a PR for some trivial nits that are virtually same before/after in general given that we can just fix such nits later when we touch the codes for a fix together, and it uses the limited resources in Jenkins which abort the jobs globally when the number of jobs is too high for some reasons.
   
   At least this PR fixes all style nits under connection packages non-invasively so I guess it's fine but let's avoid next time.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-652086840


   **[Test build #124653 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/124653/testReport)** for PR 28945 at commit [`e5f3562`](https://github.com/apache/spark/commit/e5f356263f757ee02454f6d1a8db52244a7a46d9).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651147605


   This has been a known convention and I also have been encouraging to follow the rule in new PRs. However, I don't think this post-mortem PR is worth because the existing code was made by multiple commits. I usually recommend to use the guideline when the author touches that part, @gaborgsomogyi .


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-652087903






----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] gaborgsomogyi commented on pull request #28945: [MINOR][SQL] Fix spaces in JDBC connection providers

Posted by GitBox <gi...@apache.org>.
gaborgsomogyi commented on pull request #28945:
URL: https://github.com/apache/spark/pull/28945#issuecomment-651178585


   OK @dongjoon-hyun , agreed. Should we proceed w/ this PR or should we melt them together?


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org