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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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
45 matches
Mail list logo