You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@beam.apache.org by Maximilian Michels <mx...@apache.org> on 2019/07/09 20:35:41 UTC
Re: Spotless exclusions
Thanks for asking here on the mailing list. Just saw the PR.
The PR breaks Spotless for the Flink Runner, although all of its source is under `src/`. The Flink Runner uses multiple source directories for supporting different Flink versions. Not all of them seem to be recognized anymore now:
runners/flink/src/... <-- Spotless does not work
runners/flink/1.5/src/... <-- Spotless works
I prefer Anton's changes over the previous solution, but I couldn't get it to work with the Flink project layout. I'll experiment but otherwise I would propose to revert to the old solution.
Cheers,
Max
On 27.06.19 01:50, Lukasz Cwik wrote:
>
> On Wed, Jun 26, 2019 at 4:22 PM Anton Kedin <kedin@google.com
> <ma...@google.com>> wrote:
>
> Currently our spotless is configured globally [1] (for java at
> least) to include all source files by '**/*.java'. And then we
> exclude things explicitly. Don't know why, but these exclusions are
> ignored for me sometimes, for example `./gradlew
> :sdks:java:core:spotlessJavaCheck` always fails when checking the
> generated files under
> `.../build/generated-src/antlr/main/org/apache/beam/sdk/schemas/parser/generated`.
>
> Few questions:
> * can someone point me to a discussion or a jira about this behavior?
>
>
> BEAM-6399 and BEAM-7366 allude to something wonky going on.
>
>
> * do we actually have a use case of checking the source files that
> are not under 'src'?
>
>
> No
>
>
> * if not, can we switch the config to only check for sources under
> 'src' [2]?
>
>
> Yes
>
>
> * alternatively, would it make sense to introduce project-specific
> overrides?
>
>
> All src should be under src/ so it is unlikely to be useful.
>
>
>
> [1] https://github.com/apache/beam/blob/af9362168606df9ec11319fe706b72466413798c/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy#L819
> [2] https://github.com/apache/beam/pull/8954
>
Re: Spotless exclusions
Posted by Maximilian Michels <mx...@apache.org>.
Quick update. I ended up removing the Flink-specific SourceSet traversal in BeamModulePlugin in favor of an explicit override in the Flink Gradle config: https://github.com/apache/beam/pull/9176
Overall, the default Spotless configuration is now much cleaner. Thank you Anton!
-Max
On 09.07.19 22:35, Maximilian Michels wrote:
> Thanks for asking here on the mailing list. Just saw the PR.
>
> The PR breaks Spotless for the Flink Runner, although all of its source is under `src/`. The Flink Runner uses multiple source directories for supporting different Flink versions. Not all of them seem to be recognized anymore now:
>
> runners/flink/src/... <-- Spotless does not work
> runners/flink/1.5/src/... <-- Spotless works
>
> I prefer Anton's changes over the previous solution, but I couldn't get it to work with the Flink project layout. I'll experiment but otherwise I would propose to revert to the old solution.
>
> Cheers,
> Max
>
> On 27.06.19 01:50, Lukasz Cwik wrote:
> >
> > On Wed, Jun 26, 2019 at 4:22 PM Anton Kedin <kedin@google.com
> > <ma...@google.com>> wrote:
> >
> > Currently our spotless is configured globally [1] (for java at
> > least) to include all source files by '**/*.java'. And then we
> > exclude things explicitly. Don't know why, but these exclusions are
> > ignored for me sometimes, for example `./gradlew
> > :sdks:java:core:spotlessJavaCheck` always fails when checking the
> > generated files under
> > `.../build/generated-src/antlr/main/org/apache/beam/sdk/schemas/parser/generated`.
> >
> > Few questions:
> > * can someone point me to a discussion or a jira about this behavior?
> >
> >
> > BEAM-6399 and BEAM-7366 allude to something wonky going on.
> >
> >
> > * do we actually have a use case of checking the source files that
> > are not under 'src'?
> >
> >
> > No
> >
> >
> > * if not, can we switch the config to only check for sources under
> > 'src' [2]?
> >
> >
> > Yes
> >
> >
> > * alternatively, would it make sense to introduce project-specific
> > overrides?
> >
> >
> > All src should be under src/ so it is unlikely to be useful.
> >
> >
> >
> > [1] https://github.com/apache/beam/blob/af9362168606df9ec11319fe706b72466413798c/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy#L819
> > [2] https://github.com/apache/beam/pull/8954
> >
>