Re: Spotless exclusions

[Maximilian Michels]

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

[Maximilian Michels]

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

[Lukasz Cwik]

On Wed, Jun 26, 2019 at 4:22 PM Anton Kedin  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
>

Spotless exclusions

[Anton Kedin]

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?
 * do we actually have a use case of checking the source files that are not
under 'src'?
 * if not, can we switch the config to only check for sources under 'src'
[2]?
 * alternatively, would it make sense to introduce project-specific
overrides?

[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