[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-25 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/12555 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is ena

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-25 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-214183674 I just ran into this issue again (pull requests failing style test). I'm going to merge this. Let's see how it goes ... we can always disable it if it ends up too annoying

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213207578 Ah, that syntax seems complicated enough to not be worth it (probably no-one will remember how to set it). I think that for those with workflows that conflict wit

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread jodersky
Github user jodersky commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213185109 Fair enough, test:compile is alright too. I would just avoid compile:compile On Apr 21, 2016 5:48 PM, "Reynold Xin" wrote: > hm most people don't run packa

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213184473 hm most people don't run package and just compile or test:compile, so adding it there would reduce the gain. --- If your project is set up for it, you can reply to this

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread jodersky
Github user jodersky commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213176921 Hmm, maybe you need to add the project or config too? As in "set lintOnCompile in project in Config := true"? Sbt isn't too friendly when setting command line options

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213165122 @jodersky Could you get the validate task to run for things like `~sql/test-only` though? Seems like it would be outside the task dag of most common operations, which wou

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread jodersky
Github user jodersky commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213159613 As a follow up to @vanzin's comment: > It'd be nice if you could disable or enable the checks inside an existing sbt session, instead of having to exit sbt and

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread jodersky
Github user jodersky commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213157715 Thanks for looking into this, I constantly forget to run the style checker. Since you change the dependencies of `compile`, it would be great to still have a t

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213128622 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your projec

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213128623 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213128380 That's right, you need to run `reload` to pick up the new changes to the xml. And for the runtime toggle, I think maybe keep it simple for now, since you can always scala

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213128344 **[Test build #56579 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56579/consoleFull)** for PR 12555 at commit [`4f2cd22`](https://g

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread vanzin
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213125258 It's unfortunate that you need so much code for something that should be simple, but well. Only minor issues I see are: - IIUC, changing the scalastyle configura

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213121109 Yeah, I just double checked those cases and FileFunction.cached handles it correctly. Also FWIW we've been using this rule internally at databricks for a while, and it's

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread vanzin
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213119961 Ignoring the complexity argument for a bit, does the caching work correctly when you add or remove files from the repo? i.e. are new files checked and removed files igno

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213119021 Most of the code in `cachedScalaStyle` is just the boilerplate needed to call scalastyle manually. The caching code is only a few lines supporting the FileFunctions.cache

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread srowen
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213100673 This just seems like a lot of complexity, versus just adding style checks as warnings -- or am I misunderstanding what it would take to just enable that? it seems like a

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread vanzin
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213101873 I think part of the complexity is because of the cached task... I'm not exactly sure I understand what it's doing. Is it just avoiding re-building the list of files on e

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213098287 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your projec

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213098289 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213097947 **[Test build #56567 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56567/consoleFull)** for PR 12555 at commit [`8b88370`](https://g

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213090813 **[Test build #56579 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56579/consoleFull)** for PR 12555 at commit [`4f2cd22`](https://gi

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-21304 @vanzin scalastyle has this option to not fail the task on error. The problem I ran into with this was with SBT task caching, which hides the warnings if you ever compile

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread srowen
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213070653 I like the idea of just warning, rather than making the rules slightly different. It's simpler. I suppose it only benefits those who are kind of looking for style failur

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread vanzin
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213067543 The maven plugin has a `failOnViolation` configuration; I assume the sbt one has something similar. Maybe just setting that to `false` during test or compile? Then peopl

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213063456 Same with import ordering. We can probably turn those to WARN on compile step. Trying to figure out how to do that without forking the entire xml config... --- If your

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread vanzin
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213058828 > There is an environment variable here that you can set in your profile Yeah, I saw that, but I don't start sbt every time I run something, it's just running in

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213058840 The println() is a good point. Let me see if I can turn that to a warn just in the compile check. --- If your project is set up for it, you can reply to this email and h

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213058313 There is an environment variable here that you can set in your profile to never trigger this. --- If your project is set up for it, you can reply to this email and have y

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread vanzin
Github user vanzin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213055878 I'm not strongly opposed, it's just that when debugging things I sometimes add a bunch of "println" statements in the code to trace what's going on. Having the style che

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213046520 I believe @vanzin was the main person that didn't like this. Copy him here just in case. --- If your project is set up for it, you can reply to this email and have your

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213045255 **[Test build #56567 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56567/consoleFull)** for PR 12555 at commit [`8b88370`](https://gi

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread ericl
Github user ericl commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213044120 @rxin @srowen I added a check for `NOLINT_ON_COMPILE` in the environment, which disables this check if set. The user can add this to their shell profile if they want. I c

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213038221 Would it be possible to disable this via some other thing? Then we satisfy both worlds. I just took a look at the pr backlog and almost every pr failed style test once!

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread srowen
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213026077 I'm not very against it (though Maven should ideally change to check style as well on 'pre-compile'). I remember someone arguing that they didn't want to have to pass al

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213025184 Yea I think the main thing is that this will be more integrated and more automated. Many pull requests when they were first submitted fail our style checker, both from lon

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread srowen
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213023963 (Can it be done without all this code though?) You can run style checks before submitting of course, and they do get run by the PR builder. Does it change much except

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-21 Thread rxin
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-213021785 I was concerned about this increasing the time but last night I had 3 prs that failed style checkers and now do think it would've been great if the checking was more autom

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-20 Thread srowen
Github user srowen commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-212767129 I'm not sure about that; it's a separable idea and phase. In Maven, this would generally happen in "verify" phase after "compile". In fact we've had the opposite complai

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-20 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-212745302 Merged build finished. Test PASSed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your projec

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-20 Thread AmplabJenkins
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-212745308 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-20 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-212744664 **[Test build #56463 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56463/consoleFull)** for PR 12555 at commit [`403fab6`](https://g

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-20 Thread SparkQA
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12555#issuecomment-212708426 **[Test build #56463 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56463/consoleFull)** for PR 12555 at commit [`403fab6`](https://gi

[GitHub] spark pull request: [SPARK-14790] Always run scalastyle on sbt com...

2016-04-20 Thread ericl
GitHub user ericl opened a pull request: https://github.com/apache/spark/pull/12555 [SPARK-14790] Always run scalastyle on sbt compile and test ## What changes were proposed in this pull request? Sbt compile and test should also run scalastyle. This makes it less likely you