[DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

Hi,


The context is that we currently have 3 checks in the build:

- Checkstyle,

- Eclipse-Warnings,

- RAT


CheckStyle and RAT are executed with almost every target we run: build,
jar, test, test-some, testclasslist, etc.; on the other hand,
Eclipse-Warnings is executed automatically only with the artifacts target.


Checkstyle currently uses some caching, so subsequent reruns without
cleaning the project validate only the modified files.


Both CI - Jenkins and Circle forces running all checks.


I want to discuss whether you are ok with extracting all checks to their
distinct target and not running it automatically with the targets which
devs usually run locally. In particular:



   - "build", "jar", and all "test" targets would not trigger CheckStyle,
   RAT or Eclipse-Warnings
   - A new target "check" would trigger all CheckStyle, RAT, and
   Eclipse-Warnings
   - The new "check" target would be run along with the "artifacts" target
   on Jenkins-CI, and it as a separate build step in CircleCI


The rationale for that change is:

   - Running all the checks together would be more consistent, but running
   all of them automatically with build and test targets could waste time when
   we develop something locally, frequently rebuilding and running tests.
   - On the other hand, it would be more consistent if the build did what
   we want - as a dev, when prototyping, I don't want to be forced to run
   analysis (and potentially fix issues) whenever I want to build a project or
   just run a single test.
   - There are ways to avoid running checks automatically by specifying
   some build properties. Though, the discussion is about the default behavior
   - on the flip side, if one wants to run the checks along with the specified
   target, they could add the "check" target to the command line.


The rationale for keeping the checks running automatically with every
target is to reduce the likelihood of not running the checks locally before
pushing the branch and being surprised by failing CI soon after starting
the build.


That could be fixed by running checks in a pre-push Git hook. There are
some benefits of this compared to the current behavior:

   - the checks would be run automatically only once
   - they would be triggered even for those devs who do everything in IDE
   and do not even touch Ant commands directly


Checks can take time; to optimize that, they could be enforced locally to
verify only the modified files in the same way as we currently determine
the tests to be repeated for CircleCI.


Thanks
- - -- --- ----- -------- -------------
Jacek Lewandowski

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

Thanks,

I will follow that path then,



pon., 10 lip 2023 o 19:03 Jon Meredith <jo...@apache.org> napisał(a):

> +1 from me too. I would support removing all of the optional checks from
> jar/test as I also hit issues with rat from time to time while iterating,
> as long as the CI system runs them and makes it very clear for any
> committer there are failures.
>
> On Mon, Jul 10, 2023 at 9:40 AM Josh McKenzie <jm...@apache.org>
> wrote:
>
>>
>>    - Remove the checkstyle dependency from "jar" and "test"
>>    - Create a single "check" target that includes all the checks we
>>    expect to pass in the CI (currently Checkstyle, RAT, and Eclipse-Warnings),
>>    making this task the default.
>>
>> +1 here.
>>
>> (of note: haven't forgotten the request from this thread to share local
>> env; just gotten sidetracked by things and also realized how little I've
>> actually modified locally since I just run most of the linting against
>> delta'ed files only to keep my changed work in compliance. Still a very
>> noisy mess when SpotBugs is run against the entire codebase proper)
>>
>> On Mon, Jul 10, 2023, at 7:13 AM, Brandon Williams wrote:
>>
>> On Mon, Jul 10, 2023 at 6:07 AM Jacek Lewandowski
>> <le...@gmail.com> wrote:
>> > Remove the checkstyle dependency from "jar" and "test"
>> > Create a single "check" target that includes all the checks we expect
>> to pass in the CI (currently Checkstyle, RAT, and Eclipse-Warnings), making
>> this task the default.
>>
>> I support this.  Having checkstyle run when building is clearly
>> constant friction for many, even though you can disable it.
>>
>>
>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jon Meredith <jo...@apache.org>]

+1 from me too. I would support removing all of the optional checks from
jar/test as I also hit issues with rat from time to time while iterating,
as long as the CI system runs them and makes it very clear for any
committer there are failures.

On Mon, Jul 10, 2023 at 9:40 AM Josh McKenzie <jm...@apache.org> wrote:

>
>    - Remove the checkstyle dependency from "jar" and "test"
>    - Create a single "check" target that includes all the checks we
>    expect to pass in the CI (currently Checkstyle, RAT, and Eclipse-Warnings),
>    making this task the default.
>
> +1 here.
>
> (of note: haven't forgotten the request from this thread to share local
> env; just gotten sidetracked by things and also realized how little I've
> actually modified locally since I just run most of the linting against
> delta'ed files only to keep my changed work in compliance. Still a very
> noisy mess when SpotBugs is run against the entire codebase proper)
>
> On Mon, Jul 10, 2023, at 7:13 AM, Brandon Williams wrote:
>
> On Mon, Jul 10, 2023 at 6:07 AM Jacek Lewandowski
> <le...@gmail.com> wrote:
> > Remove the checkstyle dependency from "jar" and "test"
> > Create a single "check" target that includes all the checks we expect to
> pass in the CI (currently Checkstyle, RAT, and Eclipse-Warnings), making
> this task the default.
>
> I support this.  Having checkstyle run when building is clearly
> constant friction for many, even though you can disable it.
>
>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Josh McKenzie <jm...@apache.org>]

>  • Remove the checkstyle dependency from "jar" and "test"
>  • Create a single "check" target that includes all the checks we expect to pass in the CI (currently Checkstyle, RAT, and Eclipse-Warnings), making this task the default.
+1 here.

(of note: haven't forgotten the request from this thread to share local env; just gotten sidetracked by things and also realized how little I've actually modified locally since I just run most of the linting against delta'ed files only to keep my changed work in compliance. Still a very noisy mess when SpotBugs is run against the entire codebase proper)

On Mon, Jul 10, 2023, at 7:13 AM, Brandon Williams wrote:
> On Mon, Jul 10, 2023 at 6:07 AM Jacek Lewandowski
> <le...@gmail.com> wrote:
> > Remove the checkstyle dependency from "jar" and "test"
> > Create a single "check" target that includes all the checks we expect to pass in the CI (currently Checkstyle, RAT, and Eclipse-Warnings), making this task the default.
> 
> I support this.  Having checkstyle run when building is clearly
> constant friction for many, even though you can disable it.
> 

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Brandon Williams <dr...@gmail.com>]

On Mon, Jul 10, 2023 at 6:07 AM Jacek Lewandowski
<le...@gmail.com> wrote:
> Remove the checkstyle dependency from "jar" and "test"
> Create a single "check" target that includes all the checks we expect to pass in the CI (currently Checkstyle, RAT, and Eclipse-Warnings), making this task the default.

I support this.  Having checkstyle run when building is clearly
constant friction for many, even though you can disable it.

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

Maxim, I don't think it would work, especially this command:


"ant test -Dno-build=true"


would execute the whole pipeline up to the "test" target, skipping only the
"build" target. However, none of its dependencies would be missed. In Ant,
when a target is skipped due to some property, skipping applies only to
that target, but its dependencies are executed as usual.


Also, I started this discussion because the commands we execute locally
should be short, simple, easily memorable, and self-explanatory. We want
everybody to refrain from digging into the build.xml to learn flags.


Please correct me if I'm wrong, but I believe this is what most of us do
from the terminal:

- cleaning,

- building jars,

- running checks (currently implicitly by building or testing),

- running tests of a single method / test-class

- generating IDE configuration


Everyone has excellent ideas, but those suggestions do not converge into a
single solution. That's why I'd like to make that simple change that would
reduce flags' use in everyday development. Concretely, can we get an
agreement to:

   - Remove the checkstyle dependency from "jar" and "test"
   - Create a single "check" target that includes all the checks we expect
   to pass in the CI (currently Checkstyle, RAT, and Eclipse-Warnings), making
   this task the default.


?


thanks,

Jacek

czw., 6 lip 2023 o 19:55 Jon Meredith <jo...@apache.org> napisał(a):

> sorry, hit send early.
>
> ant test is an interesting one as it seems impractical to run all tests
> sequentially, but somebody may want to I suppose.
>
> On Thu, Jul 6, 2023 at 11:53 AM Jon Meredith <jo...@apache.org>
> wrote:
>
>> I think the -Dno-blah settings have usability issues. As they look at
>> the property name, not the value, you cannot override them or default
>> them with ANT_ARGS or by importing to another ant build file.  The way
>> rat.skip does it seems much better using configured value.
>>
>> Ideally, I would like an easy/fast configuration to set a default for
>> checks that slow up the compilation/test cycle locally to be able to
>> iterate quickly compile and deal with javadoc/checkstyle comments when
>> they're ready to commit, or opt into them on the commandline when
>> needed.
>>
>> e.g.
>> export ANT_ARGS="-Dcheckstyle.default=skip -Djavadoc.default=skip"
>> ant             # should just compile, no checkstyle/javadoc etc
>> ant checkstyle  # explicitly requested, run checkstyle
>>
>> Similarly I'd like to have the option to configure any CI system I run so
>> all
>> non-execution essential checks run in their own pipeline and fail the
>> build if there's a problem, but still run the other test targets despite
>> violations. Each builder wasted the time running the checks that only
>> need to happen once and you didn't get feedback about your tests that
>> could have run. Of course not everybody may want that and the main
>> Apache Cassandra CI may only want to run tests for checked commits
>> for resource reasons.
>>
>> Also,as a minor nuisance, if you forget the =true as in the examples,
>> ant consumes the next argument as the value, so "ant publish
>> -Dno-tests -Dno-checks" would set no-tests=-Dno-checks and run the
>> checks you tried to skip anyway.
>>
>> Back to the proposal, I like the idea of an explicit check target that
>> runs all checks,
>> I would not personally have the default target run them but think that's
>> fine as long
>> as you can disable them.
>>
>> ant test is an interesting one
>>
>> On Thu, Jul 6, 2023 at 7:30 AM Maxim Muzafarov <mm...@apache.org> wrote:
>>
>>> In my humble opinion, it is better to have only one plain and
>>> straightforward build pipeline for the whole project, with custom
>>> flags used to skip a particular step, than to have multiple pipelines
>>> under the ant tool with multiple endpoints accordingly. I mean, all
>>> the steps need to be lined up, with each step in the pipeline
>>> executing everything that stands before it unless skip flags are
>>> specified. Meanwhile, I like your idea of grouping all the checks
>>> under the dedicated step (and changing the no-checkstyle flag to
>>> no-checks accordingly as Ekaterina mentioned).
>>>
>>>
>>> Let me share a simple example of what I'm talking about with one
>>> single endpoint.
>>> Let's assume the following step order:
>>>
>>> init -> _build_java (compile) -> checks -> build -> jar -> test ->
>>> artifacts -> publish;
>>>
>>> So, the use would be:
>>>
>>> ant jar -Dno-checks
>>> ant test -Dno-build
>>> ant publish -Dno-tests -Dno-checks
>>>
>>>
>>> I'm not saying what you've proposed is bad, in fact, we're not
>>> currently doing the pipeline I'm talking about, but adding an
>>> additional endpoint is something we should consider very carefully as
>>> it may create some difficulties for Maven/Gradle migration if it ever
>>> happens.
>>>
>>> So, if I'm not mistaken the following you're trying to add a new
>>> endpoint to the way how we might build the project:
>>>
>>> - "ant [check]" = build + all checks (first endpoint)
>>> - "ant jar" = build + make jars + no checks (second endpoint)
>>>
>>> And I would suggest running `ant jar -Dno-checks` instead to achieve
>>> the same result assuming the `jar` is still transitively dependent on
>>> `checks`.
>>>
>>> On Thu, 6 Jul 2023 at 14:02, Jacek Lewandowski
>>> <le...@gmail.com> wrote:
>>> >
>>> > Great discussion, but I feel we still have no conclusion.
>>> >
>>> >
>>> > I fully support automatically setting up IDE(A) to run the necessary
>>> stuff automatically in a developer-friendly environment, but let it be
>>> continued in a separate thread.
>>> >
>>> >
>>> > I wouldn't say I like flags, especially if they have to be used on a
>>> daily basis. The build script help message does not list them when "ant -p"
>>> is run.
>>> >
>>> >
>>> > I'm going to make these changes unless it is vetoed:
>>> >
>>> > "ant [check]" = build + all checks, build everything, and run all the
>>> checks; also, this would become the default target if no target is specified
>>> > "ant jar" = build + make jars: build all the jars and tests, no checks
>>> > All "test" commands = build + make jars + run the tests: build all the
>>> jars and tests, run the tests, no checks
>>> >
>>> >
>>> > Therefore, a user who wants to validate their branch before running CI
>>> would need to run just "ant" without any args. This way, a newcomer who
>>> does not know our build targets will likely run the checks.
>>> >
>>> >
>>> > We still need some flags for skipping specific tasks to optimize for
>>> CI, but in general, they would not be required for local development.
>>> >
>>> >
>>> > Flags will also be needed to customize some tasks, but they should be
>>> optional for newcomers. In addition, a "help" target could display a list
>>> of selected tasks and properties with descriptions.
>>> >
>>> >
>>> > I'd be more than happy if we could conclude the discussion somehow and
>>> move forward :)
>>> >
>>> >
>>> > thanks,
>>> >
>>> > Jacek
>>> >
>>> >
>>> >
>>> > czw., 29 cze 2023 o 23:34 Ekaterina Dimitrova <e....@gmail.com>
>>> napisał(a):
>>> >>
>>> >> There is a separate thread started and respective ticket for
>>> generate-idea-files.
>>> >> https://lists.apache.org/thread/o2fdkyv2skvf9ngy9jhpnhvo92qvr17m
>>> >> CASSANDRA-18467
>>> >>
>>> >>
>>> >> On Thu, 29 Jun 2023 at 16:54, Jeremiah Jordan <
>>> jeremiah.jordan@gmail.com> wrote:
>>> >>>
>>> >>> +100 I support making generate-idea-files auto setup everything in
>>> IntelliJ for you.  If you post a diff, I will test it.
>>> >>>
>>> >>> On this proposal, I don’t really have an opinion one way or the
>>> other about what the default is for local "ant jar”, if its slow I will
>>> figure out how to turn it off, if its fast I will leave it on.
>>> >>> I do care that CI runs checks, and complains loudly if something is
>>> wrong such that it is very easy to tell during review.
>>> >>>
>>> >>> -Jeremiah
>>> >>>
>>> >>> On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie <jm...@apache.org>
>>> wrote:
>>> >>>>
>>> >>>> In accord I added an opt-out for each hook, and will require such
>>> here as well
>>> >>>>
>>> >>>> On for main branches, off for feature branches seems like it might
>>> blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches
>>> means style checks and build on hook across those branches" which isn't
>>> ideal. I don't think style check failures after push upstream are frequent
>>> enough to make the cost/benefit there make sense overall are they?
>>> >>>>
>>> >>>> Related to this - I have sonarlint, spotbugs, and checkstyle all
>>> running inside idea; since pulling those in and tuning the configs a bit I
>>> haven't run into a single issue w/our checkstyle build target (go figure).
>>> Having the required style checks reflected realtime inside your work
>>> environment goes a long way towards making it a more intuitive part of your
>>> workflow rather than being an annoying last minute block of your ability to
>>> progress that requires circling back into the code.
>>> >>>>
>>> >>>> From a technical perspective, it looks like adding a reference
>>> "externalDependencies.xml" to our ide/idea directory which we copied over
>>> during "generate-idea-files" would be sufficient to get idea to pop up
>>> prompts to install those extensions if you don't have them when opening the
>>> project (theory; haven't tested).
>>> >>>>
>>> >>>> We'd need to make sure the configuration for each of those was
>>> calibrated to our project out of the box of course, but making style
>>> considerations a first-class citizen in that way seems a more intuitive and
>>> human-centered approach to all this rather than debating nuance of our
>>> command-line targets, hooks, and how we present things to people. To
>>> Berenguer's point - better to have these be completely invisible to people
>>> with their workflows and Just Work (except for when your IDE scolds you for
>>> bad behavior w/build errors immediately).
>>> >>>>
>>> >>>> I still think Flags Are Bad. :)
>>> >>>>
>>> >>>> On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
>>> >>>>
>>> >>>> Should we just keep a consolidated for all kind of checks no-check
>>> flag and get rid of the no-checkstyle one?
>>> >>>>
>>> >>>> Trading one for one with Josh :-)
>>> >>>>
>>> >>>> Best regards,
>>> >>>> Ekaterina
>>> >>>>
>>> >>>> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org>
>>> wrote:
>>> >>>>
>>> >>>>
>>> >>>> I really prefer separate tasks than flags. Flags are not listed in
>>> the help message like "ant -p" and are not auto-completed in the terminal.
>>> That makes them almost undiscoverable for newcomers.
>>> >>>>
>>> >>>> Please, no more flags. We are more than flaggy enough right now.
>>> >>>>
>>> >>>> Having to dig through build.xml to determine how to change things
>>> or do things is painful; the more we can avoid this (for oldtimers and
>>> newcomers alike!) the better.
>>> >>>>
>>> >>>> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <
>>> lewandowski.jacek@gmail.com> wrote:
>>> >>>>
>>> >>>> There is another target called "build", which retrieves
>>> dependencies, and then calls "build-project".
>>> >>>>
>>> >>>>
>>> >>>>
>>> >>>> Is it intended to be called by a user ?
>>> >>>>
>>> >>>> If not, please follow the ant style prefixing the target name with
>>> an underscore (so that it does not appear in the `ant -projecthelp` list).
>>> >>>>
>>> >>>> If possible, I agree with Brandon, `build` is the better name to
>>> expose to the user.
>>> >>>>
>>> >>>>
>>> >>>>
>>>
>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jon Meredith <jo...@apache.org>]

sorry, hit send early.

ant test is an interesting one as it seems impractical to run all tests
sequentially, but somebody may want to I suppose.

On Thu, Jul 6, 2023 at 11:53 AM Jon Meredith <jo...@apache.org> wrote:

> I think the -Dno-blah settings have usability issues. As they look at
> the property name, not the value, you cannot override them or default
> them with ANT_ARGS or by importing to another ant build file.  The way
> rat.skip does it seems much better using configured value.
>
> Ideally, I would like an easy/fast configuration to set a default for
> checks that slow up the compilation/test cycle locally to be able to
> iterate quickly compile and deal with javadoc/checkstyle comments when
> they're ready to commit, or opt into them on the commandline when
> needed.
>
> e.g.
> export ANT_ARGS="-Dcheckstyle.default=skip -Djavadoc.default=skip"
> ant             # should just compile, no checkstyle/javadoc etc
> ant checkstyle  # explicitly requested, run checkstyle
>
> Similarly I'd like to have the option to configure any CI system I run so
> all
> non-execution essential checks run in their own pipeline and fail the
> build if there's a problem, but still run the other test targets despite
> violations. Each builder wasted the time running the checks that only
> need to happen once and you didn't get feedback about your tests that
> could have run. Of course not everybody may want that and the main
> Apache Cassandra CI may only want to run tests for checked commits
> for resource reasons.
>
> Also,as a minor nuisance, if you forget the =true as in the examples,
> ant consumes the next argument as the value, so "ant publish
> -Dno-tests -Dno-checks" would set no-tests=-Dno-checks and run the
> checks you tried to skip anyway.
>
> Back to the proposal, I like the idea of an explicit check target that
> runs all checks,
> I would not personally have the default target run them but think that's
> fine as long
> as you can disable them.
>
> ant test is an interesting one
>
> On Thu, Jul 6, 2023 at 7:30 AM Maxim Muzafarov <mm...@apache.org> wrote:
>
>> In my humble opinion, it is better to have only one plain and
>> straightforward build pipeline for the whole project, with custom
>> flags used to skip a particular step, than to have multiple pipelines
>> under the ant tool with multiple endpoints accordingly. I mean, all
>> the steps need to be lined up, with each step in the pipeline
>> executing everything that stands before it unless skip flags are
>> specified. Meanwhile, I like your idea of grouping all the checks
>> under the dedicated step (and changing the no-checkstyle flag to
>> no-checks accordingly as Ekaterina mentioned).
>>
>>
>> Let me share a simple example of what I'm talking about with one
>> single endpoint.
>> Let's assume the following step order:
>>
>> init -> _build_java (compile) -> checks -> build -> jar -> test ->
>> artifacts -> publish;
>>
>> So, the use would be:
>>
>> ant jar -Dno-checks
>> ant test -Dno-build
>> ant publish -Dno-tests -Dno-checks
>>
>>
>> I'm not saying what you've proposed is bad, in fact, we're not
>> currently doing the pipeline I'm talking about, but adding an
>> additional endpoint is something we should consider very carefully as
>> it may create some difficulties for Maven/Gradle migration if it ever
>> happens.
>>
>> So, if I'm not mistaken the following you're trying to add a new
>> endpoint to the way how we might build the project:
>>
>> - "ant [check]" = build + all checks (first endpoint)
>> - "ant jar" = build + make jars + no checks (second endpoint)
>>
>> And I would suggest running `ant jar -Dno-checks` instead to achieve
>> the same result assuming the `jar` is still transitively dependent on
>> `checks`.
>>
>> On Thu, 6 Jul 2023 at 14:02, Jacek Lewandowski
>> <le...@gmail.com> wrote:
>> >
>> > Great discussion, but I feel we still have no conclusion.
>> >
>> >
>> > I fully support automatically setting up IDE(A) to run the necessary
>> stuff automatically in a developer-friendly environment, but let it be
>> continued in a separate thread.
>> >
>> >
>> > I wouldn't say I like flags, especially if they have to be used on a
>> daily basis. The build script help message does not list them when "ant -p"
>> is run.
>> >
>> >
>> > I'm going to make these changes unless it is vetoed:
>> >
>> > "ant [check]" = build + all checks, build everything, and run all the
>> checks; also, this would become the default target if no target is specified
>> > "ant jar" = build + make jars: build all the jars and tests, no checks
>> > All "test" commands = build + make jars + run the tests: build all the
>> jars and tests, run the tests, no checks
>> >
>> >
>> > Therefore, a user who wants to validate their branch before running CI
>> would need to run just "ant" without any args. This way, a newcomer who
>> does not know our build targets will likely run the checks.
>> >
>> >
>> > We still need some flags for skipping specific tasks to optimize for
>> CI, but in general, they would not be required for local development.
>> >
>> >
>> > Flags will also be needed to customize some tasks, but they should be
>> optional for newcomers. In addition, a "help" target could display a list
>> of selected tasks and properties with descriptions.
>> >
>> >
>> > I'd be more than happy if we could conclude the discussion somehow and
>> move forward :)
>> >
>> >
>> > thanks,
>> >
>> > Jacek
>> >
>> >
>> >
>> > czw., 29 cze 2023 o 23:34 Ekaterina Dimitrova <e....@gmail.com>
>> napisał(a):
>> >>
>> >> There is a separate thread started and respective ticket for
>> generate-idea-files.
>> >> https://lists.apache.org/thread/o2fdkyv2skvf9ngy9jhpnhvo92qvr17m
>> >> CASSANDRA-18467
>> >>
>> >>
>> >> On Thu, 29 Jun 2023 at 16:54, Jeremiah Jordan <
>> jeremiah.jordan@gmail.com> wrote:
>> >>>
>> >>> +100 I support making generate-idea-files auto setup everything in
>> IntelliJ for you.  If you post a diff, I will test it.
>> >>>
>> >>> On this proposal, I don’t really have an opinion one way or the other
>> about what the default is for local "ant jar”, if its slow I will figure
>> out how to turn it off, if its fast I will leave it on.
>> >>> I do care that CI runs checks, and complains loudly if something is
>> wrong such that it is very easy to tell during review.
>> >>>
>> >>> -Jeremiah
>> >>>
>> >>> On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie <jm...@apache.org>
>> wrote:
>> >>>>
>> >>>> In accord I added an opt-out for each hook, and will require such
>> here as well
>> >>>>
>> >>>> On for main branches, off for feature branches seems like it might
>> blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches
>> means style checks and build on hook across those branches" which isn't
>> ideal. I don't think style check failures after push upstream are frequent
>> enough to make the cost/benefit there make sense overall are they?
>> >>>>
>> >>>> Related to this - I have sonarlint, spotbugs, and checkstyle all
>> running inside idea; since pulling those in and tuning the configs a bit I
>> haven't run into a single issue w/our checkstyle build target (go figure).
>> Having the required style checks reflected realtime inside your work
>> environment goes a long way towards making it a more intuitive part of your
>> workflow rather than being an annoying last minute block of your ability to
>> progress that requires circling back into the code.
>> >>>>
>> >>>> From a technical perspective, it looks like adding a reference
>> "externalDependencies.xml" to our ide/idea directory which we copied over
>> during "generate-idea-files" would be sufficient to get idea to pop up
>> prompts to install those extensions if you don't have them when opening the
>> project (theory; haven't tested).
>> >>>>
>> >>>> We'd need to make sure the configuration for each of those was
>> calibrated to our project out of the box of course, but making style
>> considerations a first-class citizen in that way seems a more intuitive and
>> human-centered approach to all this rather than debating nuance of our
>> command-line targets, hooks, and how we present things to people. To
>> Berenguer's point - better to have these be completely invisible to people
>> with their workflows and Just Work (except for when your IDE scolds you for
>> bad behavior w/build errors immediately).
>> >>>>
>> >>>> I still think Flags Are Bad. :)
>> >>>>
>> >>>> On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
>> >>>>
>> >>>> Should we just keep a consolidated for all kind of checks no-check
>> flag and get rid of the no-checkstyle one?
>> >>>>
>> >>>> Trading one for one with Josh :-)
>> >>>>
>> >>>> Best regards,
>> >>>> Ekaterina
>> >>>>
>> >>>> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org>
>> wrote:
>> >>>>
>> >>>>
>> >>>> I really prefer separate tasks than flags. Flags are not listed in
>> the help message like "ant -p" and are not auto-completed in the terminal.
>> That makes them almost undiscoverable for newcomers.
>> >>>>
>> >>>> Please, no more flags. We are more than flaggy enough right now.
>> >>>>
>> >>>> Having to dig through build.xml to determine how to change things or
>> do things is painful; the more we can avoid this (for oldtimers and
>> newcomers alike!) the better.
>> >>>>
>> >>>> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
>> >>>>
>> >>>>
>> >>>>
>> >>>> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <
>> lewandowski.jacek@gmail.com> wrote:
>> >>>>
>> >>>> There is another target called "build", which retrieves
>> dependencies, and then calls "build-project".
>> >>>>
>> >>>>
>> >>>>
>> >>>> Is it intended to be called by a user ?
>> >>>>
>> >>>> If not, please follow the ant style prefixing the target name with
>> an underscore (so that it does not appear in the `ant -projecthelp` list).
>> >>>>
>> >>>> If possible, I agree with Brandon, `build` is the better name to
>> expose to the user.
>> >>>>
>> >>>>
>> >>>>
>>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jon Meredith <jo...@apache.org>]

I think the -Dno-blah settings have usability issues. As they look at
the property name, not the value, you cannot override them or default
them with ANT_ARGS or by importing to another ant build file.  The way
rat.skip does it seems much better using configured value.

Ideally, I would like an easy/fast configuration to set a default for
checks that slow up the compilation/test cycle locally to be able to
iterate quickly compile and deal with javadoc/checkstyle comments when
they're ready to commit, or opt into them on the commandline when
needed.

e.g.
export ANT_ARGS="-Dcheckstyle.default=skip -Djavadoc.default=skip"
ant             # should just compile, no checkstyle/javadoc etc
ant checkstyle  # explicitly requested, run checkstyle

Similarly I'd like to have the option to configure any CI system I run so
all
non-execution essential checks run in their own pipeline and fail the
build if there's a problem, but still run the other test targets despite
violations. Each builder wasted the time running the checks that only
need to happen once and you didn't get feedback about your tests that
could have run. Of course not everybody may want that and the main
Apache Cassandra CI may only want to run tests for checked commits
for resource reasons.

Also,as a minor nuisance, if you forget the =true as in the examples,
ant consumes the next argument as the value, so "ant publish
-Dno-tests -Dno-checks" would set no-tests=-Dno-checks and run the
checks you tried to skip anyway.

Back to the proposal, I like the idea of an explicit check target that runs
all checks,
I would not personally have the default target run them but think that's
fine as long
as you can disable them.

ant test is an interesting one

On Thu, Jul 6, 2023 at 7:30 AM Maxim Muzafarov <mm...@apache.org> wrote:

> In my humble opinion, it is better to have only one plain and
> straightforward build pipeline for the whole project, with custom
> flags used to skip a particular step, than to have multiple pipelines
> under the ant tool with multiple endpoints accordingly. I mean, all
> the steps need to be lined up, with each step in the pipeline
> executing everything that stands before it unless skip flags are
> specified. Meanwhile, I like your idea of grouping all the checks
> under the dedicated step (and changing the no-checkstyle flag to
> no-checks accordingly as Ekaterina mentioned).
>
>
> Let me share a simple example of what I'm talking about with one
> single endpoint.
> Let's assume the following step order:
>
> init -> _build_java (compile) -> checks -> build -> jar -> test ->
> artifacts -> publish;
>
> So, the use would be:
>
> ant jar -Dno-checks
> ant test -Dno-build
> ant publish -Dno-tests -Dno-checks
>
>
> I'm not saying what you've proposed is bad, in fact, we're not
> currently doing the pipeline I'm talking about, but adding an
> additional endpoint is something we should consider very carefully as
> it may create some difficulties for Maven/Gradle migration if it ever
> happens.
>
> So, if I'm not mistaken the following you're trying to add a new
> endpoint to the way how we might build the project:
>
> - "ant [check]" = build + all checks (first endpoint)
> - "ant jar" = build + make jars + no checks (second endpoint)
>
> And I would suggest running `ant jar -Dno-checks` instead to achieve
> the same result assuming the `jar` is still transitively dependent on
> `checks`.
>
> On Thu, 6 Jul 2023 at 14:02, Jacek Lewandowski
> <le...@gmail.com> wrote:
> >
> > Great discussion, but I feel we still have no conclusion.
> >
> >
> > I fully support automatically setting up IDE(A) to run the necessary
> stuff automatically in a developer-friendly environment, but let it be
> continued in a separate thread.
> >
> >
> > I wouldn't say I like flags, especially if they have to be used on a
> daily basis. The build script help message does not list them when "ant -p"
> is run.
> >
> >
> > I'm going to make these changes unless it is vetoed:
> >
> > "ant [check]" = build + all checks, build everything, and run all the
> checks; also, this would become the default target if no target is specified
> > "ant jar" = build + make jars: build all the jars and tests, no checks
> > All "test" commands = build + make jars + run the tests: build all the
> jars and tests, run the tests, no checks
> >
> >
> > Therefore, a user who wants to validate their branch before running CI
> would need to run just "ant" without any args. This way, a newcomer who
> does not know our build targets will likely run the checks.
> >
> >
> > We still need some flags for skipping specific tasks to optimize for CI,
> but in general, they would not be required for local development.
> >
> >
> > Flags will also be needed to customize some tasks, but they should be
> optional for newcomers. In addition, a "help" target could display a list
> of selected tasks and properties with descriptions.
> >
> >
> > I'd be more than happy if we could conclude the discussion somehow and
> move forward :)
> >
> >
> > thanks,
> >
> > Jacek
> >
> >
> >
> > czw., 29 cze 2023 o 23:34 Ekaterina Dimitrova <e....@gmail.com>
> napisał(a):
> >>
> >> There is a separate thread started and respective ticket for
> generate-idea-files.
> >> https://lists.apache.org/thread/o2fdkyv2skvf9ngy9jhpnhvo92qvr17m
> >> CASSANDRA-18467
> >>
> >>
> >> On Thu, 29 Jun 2023 at 16:54, Jeremiah Jordan <
> jeremiah.jordan@gmail.com> wrote:
> >>>
> >>> +100 I support making generate-idea-files auto setup everything in
> IntelliJ for you.  If you post a diff, I will test it.
> >>>
> >>> On this proposal, I don’t really have an opinion one way or the other
> about what the default is for local "ant jar”, if its slow I will figure
> out how to turn it off, if its fast I will leave it on.
> >>> I do care that CI runs checks, and complains loudly if something is
> wrong such that it is very easy to tell during review.
> >>>
> >>> -Jeremiah
> >>>
> >>> On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie <jm...@apache.org>
> wrote:
> >>>>
> >>>> In accord I added an opt-out for each hook, and will require such
> here as well
> >>>>
> >>>> On for main branches, off for feature branches seems like it might
> blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches
> means style checks and build on hook across those branches" which isn't
> ideal. I don't think style check failures after push upstream are frequent
> enough to make the cost/benefit there make sense overall are they?
> >>>>
> >>>> Related to this - I have sonarlint, spotbugs, and checkstyle all
> running inside idea; since pulling those in and tuning the configs a bit I
> haven't run into a single issue w/our checkstyle build target (go figure).
> Having the required style checks reflected realtime inside your work
> environment goes a long way towards making it a more intuitive part of your
> workflow rather than being an annoying last minute block of your ability to
> progress that requires circling back into the code.
> >>>>
> >>>> From a technical perspective, it looks like adding a reference
> "externalDependencies.xml" to our ide/idea directory which we copied over
> during "generate-idea-files" would be sufficient to get idea to pop up
> prompts to install those extensions if you don't have them when opening the
> project (theory; haven't tested).
> >>>>
> >>>> We'd need to make sure the configuration for each of those was
> calibrated to our project out of the box of course, but making style
> considerations a first-class citizen in that way seems a more intuitive and
> human-centered approach to all this rather than debating nuance of our
> command-line targets, hooks, and how we present things to people. To
> Berenguer's point - better to have these be completely invisible to people
> with their workflows and Just Work (except for when your IDE scolds you for
> bad behavior w/build errors immediately).
> >>>>
> >>>> I still think Flags Are Bad. :)
> >>>>
> >>>> On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
> >>>>
> >>>> Should we just keep a consolidated for all kind of checks no-check
> flag and get rid of the no-checkstyle one?
> >>>>
> >>>> Trading one for one with Josh :-)
> >>>>
> >>>> Best regards,
> >>>> Ekaterina
> >>>>
> >>>> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org>
> wrote:
> >>>>
> >>>>
> >>>> I really prefer separate tasks than flags. Flags are not listed in
> the help message like "ant -p" and are not auto-completed in the terminal.
> That makes them almost undiscoverable for newcomers.
> >>>>
> >>>> Please, no more flags. We are more than flaggy enough right now.
> >>>>
> >>>> Having to dig through build.xml to determine how to change things or
> do things is painful; the more we can avoid this (for oldtimers and
> newcomers alike!) the better.
> >>>>
> >>>> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
> >>>>
> >>>>
> >>>>
> >>>> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <
> lewandowski.jacek@gmail.com> wrote:
> >>>>
> >>>> There is another target called "build", which retrieves dependencies,
> and then calls "build-project".
> >>>>
> >>>>
> >>>>
> >>>> Is it intended to be called by a user ?
> >>>>
> >>>> If not, please follow the ant style prefixing the target name with an
> underscore (so that it does not appear in the `ant -projecthelp` list).
> >>>>
> >>>> If possible, I agree with Brandon, `build` is the better name to
> expose to the user.
> >>>>
> >>>>
> >>>>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Maxim Muzafarov <mm...@apache.org>]

In my humble opinion, it is better to have only one plain and
straightforward build pipeline for the whole project, with custom
flags used to skip a particular step, than to have multiple pipelines
under the ant tool with multiple endpoints accordingly. I mean, all
the steps need to be lined up, with each step in the pipeline
executing everything that stands before it unless skip flags are
specified. Meanwhile, I like your idea of grouping all the checks
under the dedicated step (and changing the no-checkstyle flag to
no-checks accordingly as Ekaterina mentioned).


Let me share a simple example of what I'm talking about with one
single endpoint.
Let's assume the following step order:

init -> _build_java (compile) -> checks -> build -> jar -> test ->
artifacts -> publish;

So, the use would be:

ant jar -Dno-checks
ant test -Dno-build
ant publish -Dno-tests -Dno-checks


I'm not saying what you've proposed is bad, in fact, we're not
currently doing the pipeline I'm talking about, but adding an
additional endpoint is something we should consider very carefully as
it may create some difficulties for Maven/Gradle migration if it ever
happens.

So, if I'm not mistaken the following you're trying to add a new
endpoint to the way how we might build the project:

- "ant [check]" = build + all checks (first endpoint)
- "ant jar" = build + make jars + no checks (second endpoint)

And I would suggest running `ant jar -Dno-checks` instead to achieve
the same result assuming the `jar` is still transitively dependent on
`checks`.

On Thu, 6 Jul 2023 at 14:02, Jacek Lewandowski
<le...@gmail.com> wrote:
>
> Great discussion, but I feel we still have no conclusion.
>
>
> I fully support automatically setting up IDE(A) to run the necessary stuff automatically in a developer-friendly environment, but let it be continued in a separate thread.
>
>
> I wouldn't say I like flags, especially if they have to be used on a daily basis. The build script help message does not list them when "ant -p" is run.
>
>
> I'm going to make these changes unless it is vetoed:
>
> "ant [check]" = build + all checks, build everything, and run all the checks; also, this would become the default target if no target is specified
> "ant jar" = build + make jars: build all the jars and tests, no checks
> All "test" commands = build + make jars + run the tests: build all the jars and tests, run the tests, no checks
>
>
> Therefore, a user who wants to validate their branch before running CI would need to run just "ant" without any args. This way, a newcomer who does not know our build targets will likely run the checks.
>
>
> We still need some flags for skipping specific tasks to optimize for CI, but in general, they would not be required for local development.
>
>
> Flags will also be needed to customize some tasks, but they should be optional for newcomers. In addition, a "help" target could display a list of selected tasks and properties with descriptions.
>
>
> I'd be more than happy if we could conclude the discussion somehow and move forward :)
>
>
> thanks,
>
> Jacek
>
>
>
> czw., 29 cze 2023 o 23:34 Ekaterina Dimitrova <e....@gmail.com> napisał(a):
>>
>> There is a separate thread started and respective ticket for generate-idea-files.
>> https://lists.apache.org/thread/o2fdkyv2skvf9ngy9jhpnhvo92qvr17m
>> CASSANDRA-18467
>>
>>
>> On Thu, 29 Jun 2023 at 16:54, Jeremiah Jordan <je...@gmail.com> wrote:
>>>
>>> +100 I support making generate-idea-files auto setup everything in IntelliJ for you.  If you post a diff, I will test it.
>>>
>>> On this proposal, I don’t really have an opinion one way or the other about what the default is for local "ant jar”, if its slow I will figure out how to turn it off, if its fast I will leave it on.
>>> I do care that CI runs checks, and complains loudly if something is wrong such that it is very easy to tell during review.
>>>
>>> -Jeremiah
>>>
>>> On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie <jm...@apache.org> wrote:
>>>>
>>>> In accord I added an opt-out for each hook, and will require such here as well
>>>>
>>>> On for main branches, off for feature branches seems like it might blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches means style checks and build on hook across those branches" which isn't ideal. I don't think style check failures after push upstream are frequent enough to make the cost/benefit there make sense overall are they?
>>>>
>>>> Related to this - I have sonarlint, spotbugs, and checkstyle all running inside idea; since pulling those in and tuning the configs a bit I haven't run into a single issue w/our checkstyle build target (go figure). Having the required style checks reflected realtime inside your work environment goes a long way towards making it a more intuitive part of your workflow rather than being an annoying last minute block of your ability to progress that requires circling back into the code.
>>>>
>>>> From a technical perspective, it looks like adding a reference "externalDependencies.xml" to our ide/idea directory which we copied over during "generate-idea-files" would be sufficient to get idea to pop up prompts to install those extensions if you don't have them when opening the project (theory; haven't tested).
>>>>
>>>> We'd need to make sure the configuration for each of those was calibrated to our project out of the box of course, but making style considerations a first-class citizen in that way seems a more intuitive and human-centered approach to all this rather than debating nuance of our command-line targets, hooks, and how we present things to people. To Berenguer's point - better to have these be completely invisible to people with their workflows and Just Work (except for when your IDE scolds you for bad behavior w/build errors immediately).
>>>>
>>>> I still think Flags Are Bad. :)
>>>>
>>>> On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
>>>>
>>>> Should we just keep a consolidated for all kind of checks no-check flag and get rid of the no-checkstyle one?
>>>>
>>>> Trading one for one with Josh :-)
>>>>
>>>> Best regards,
>>>> Ekaterina
>>>>
>>>> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org> wrote:
>>>>
>>>>
>>>> I really prefer separate tasks than flags. Flags are not listed in the help message like "ant -p" and are not auto-completed in the terminal. That makes them almost undiscoverable for newcomers.
>>>>
>>>> Please, no more flags. We are more than flaggy enough right now.
>>>>
>>>> Having to dig through build.xml to determine how to change things or do things is painful; the more we can avoid this (for oldtimers and newcomers alike!) the better.
>>>>
>>>> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
>>>>
>>>>
>>>>
>>>> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <le...@gmail.com> wrote:
>>>>
>>>> There is another target called "build", which retrieves dependencies, and then calls "build-project".
>>>>
>>>>
>>>>
>>>> Is it intended to be called by a user ?
>>>>
>>>> If not, please follow the ant style prefixing the target name with an underscore (so that it does not appear in the `ant -projecthelp` list).
>>>>
>>>> If possible, I agree with Brandon, `build` is the better name to expose to the user.
>>>>
>>>>
>>>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

Great discussion, but I feel we still have no conclusion.


I fully support automatically setting up IDE(A) to run the necessary stuff
automatically in a developer-friendly environment, but let it be continued
in a separate thread.


I wouldn't say I like flags, especially if they have to be used on a daily
basis. The build script help message does not list them when "ant -p" is
run.


I'm going to make these changes unless it is vetoed:

   - "ant [check]" = build + all checks, build everything, and run all the
   checks; also, this would become the default target if no target is specified
   - "ant jar" = build + make jars: build all the jars and tests, no checks
   - All "test" commands = build + make jars + run the tests: build all the
   jars and tests, run the tests, no checks


Therefore, a user who wants to validate their branch before running CI
would need to run just "ant" without any args. This way, a newcomer who
does not know our build targets will likely run the checks.


We still need some flags for skipping specific tasks to optimize for CI,
but in general, they would not be required for local development.


Flags will also be needed to customize some tasks, but they should be
optional for newcomers. In addition, a "help" target could display a list
of selected tasks and properties with descriptions.


I'd be more than happy if we could conclude the discussion somehow and move
forward :)


thanks,

Jacek



czw., 29 cze 2023 o 23:34 Ekaterina Dimitrova <e....@gmail.com>
napisał(a):

> There is a separate thread started and respective ticket for
> generate-idea-files.
> https://lists.apache.org/thread/o2fdkyv2skvf9ngy9jhpnhvo92qvr17m
> CASSANDRA-18467
>
>
> On Thu, 29 Jun 2023 at 16:54, Jeremiah Jordan <je...@gmail.com>
> wrote:
>
>> +100 I support making generate-idea-files auto setup everything in
>> IntelliJ for you.  If you post a diff, I will test it.
>>
>> On this proposal, I don’t really have an opinion one way or the other
>> about what the default is for local "ant jar”, if its slow I will figure
>> out how to turn it off, if its fast I will leave it on.
>> I do care that CI runs checks, and complains loudly if something is wrong
>> such that it is very easy to tell during review.
>>
>> -Jeremiah
>>
>> On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie <jm...@apache.org>
>> wrote:
>>
>>> In accord I added an opt-out for each hook, and will require such here
>>> as well
>>>
>>> On for main branches, off for feature branches seems like it might
>>> blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches
>>> means style checks and build on hook across those branches" which isn't
>>> ideal. I don't think style check failures after push upstream are frequent
>>> enough to make the cost/benefit there make sense overall are they?
>>>
>>> Related to this - I have sonarlint, spotbugs, and checkstyle all running
>>> inside idea; since pulling those in and tuning the configs a bit I haven't
>>> run into a single issue w/our checkstyle build target (go figure). Having
>>> the required style checks reflected realtime inside your work environment
>>> goes a long way towards making it a more intuitive part of your workflow
>>> rather than being an annoying last minute block of your ability to progress
>>> that requires circling back into the code.
>>>
>>> From a technical perspective, it looks like adding a reference
>>> "externalDependencies.xml" to our ide/idea directory which we copied over
>>> during "generate-idea-files" would be sufficient to get idea to pop up
>>> prompts to install those extensions if you don't have them when opening the
>>> project (theory; haven't tested).
>>>
>>> We'd need to make sure the configuration for each of those was
>>> calibrated to our project out of the box of course, but making style
>>> considerations a first-class citizen in that way seems a more intuitive and
>>> human-centered approach to all this rather than debating nuance of our
>>> command-line targets, hooks, and how we present things to people. To
>>> Berenguer's point - better to have these be completely invisible to people
>>> with their workflows and Just Work (except for when your IDE scolds you for
>>> bad behavior w/build errors immediately).
>>>
>>> I still think Flags Are Bad. :)
>>>
>>> On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
>>>
>>> Should we just keep a consolidated for all kind of checks no-check flag
>>> and get rid of the no-checkstyle one?
>>>
>>> Trading one for one with Josh :-)
>>>
>>> Best regards,
>>> Ekaterina
>>>
>>> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org>
>>> wrote:
>>>
>>>
>>> I really prefer separate tasks than flags. Flags are not listed in the
>>> help message like "ant -p" and are not auto-completed in the terminal. That
>>> makes them almost undiscoverable for newcomers.
>>>
>>> Please, no more flags. We are *more* than flaggy enough right now.
>>>
>>> Having to dig through build.xml to determine how to change things or do
>>> things is painful; the more we can avoid this (for oldtimers and newcomers
>>> alike!) the better.
>>>
>>> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
>>>
>>>
>>>
>>> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <
>>> lewandowski.jacek@gmail.com> wrote:
>>>
>>> There is another target called "build", which retrieves dependencies,
>>> and then calls "build-project".
>>>
>>>
>>>
>>> Is it intended to be called by a user ?
>>>
>>> If not, please follow the ant style prefixing the target name with an
>>> underscore (so that it does not appear in the `ant -projecthelp` list).
>>>
>>> If possible, I agree with Brandon, `build` is the better name to expose
>>> to the user.
>>>
>>>
>>>
>>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Ekaterina Dimitrova <e....@gmail.com>]

There is a separate thread started and respective ticket for
generate-idea-files.
https://lists.apache.org/thread/o2fdkyv2skvf9ngy9jhpnhvo92qvr17m
CASSANDRA-18467


On Thu, 29 Jun 2023 at 16:54, Jeremiah Jordan <je...@gmail.com>
wrote:

> +100 I support making generate-idea-files auto setup everything in
> IntelliJ for you.  If you post a diff, I will test it.
>
> On this proposal, I don’t really have an opinion one way or the other
> about what the default is for local "ant jar”, if its slow I will figure
> out how to turn it off, if its fast I will leave it on.
> I do care that CI runs checks, and complains loudly if something is wrong
> such that it is very easy to tell during review.
>
> -Jeremiah
>
> On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie <jm...@apache.org> wrote:
>
>> In accord I added an opt-out for each hook, and will require such here as
>> well
>>
>> On for main branches, off for feature branches seems like it might
>> blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches
>> means style checks and build on hook across those branches" which isn't
>> ideal. I don't think style check failures after push upstream are frequent
>> enough to make the cost/benefit there make sense overall are they?
>>
>> Related to this - I have sonarlint, spotbugs, and checkstyle all running
>> inside idea; since pulling those in and tuning the configs a bit I haven't
>> run into a single issue w/our checkstyle build target (go figure). Having
>> the required style checks reflected realtime inside your work environment
>> goes a long way towards making it a more intuitive part of your workflow
>> rather than being an annoying last minute block of your ability to progress
>> that requires circling back into the code.
>>
>> From a technical perspective, it looks like adding a reference
>> "externalDependencies.xml" to our ide/idea directory which we copied over
>> during "generate-idea-files" would be sufficient to get idea to pop up
>> prompts to install those extensions if you don't have them when opening the
>> project (theory; haven't tested).
>>
>> We'd need to make sure the configuration for each of those was calibrated
>> to our project out of the box of course, but making style considerations a
>> first-class citizen in that way seems a more intuitive and human-centered
>> approach to all this rather than debating nuance of our command-line
>> targets, hooks, and how we present things to people. To Berenguer's point -
>> better to have these be completely invisible to people with their workflows
>> and Just Work (except for when your IDE scolds you for bad behavior w/build
>> errors immediately).
>>
>> I still think Flags Are Bad. :)
>>
>> On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
>>
>> Should we just keep a consolidated for all kind of checks no-check flag
>> and get rid of the no-checkstyle one?
>>
>> Trading one for one with Josh :-)
>>
>> Best regards,
>> Ekaterina
>>
>> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org> wrote:
>>
>>
>> I really prefer separate tasks than flags. Flags are not listed in the
>> help message like "ant -p" and are not auto-completed in the terminal. That
>> makes them almost undiscoverable for newcomers.
>>
>> Please, no more flags. We are *more* than flaggy enough right now.
>>
>> Having to dig through build.xml to determine how to change things or do
>> things is painful; the more we can avoid this (for oldtimers and newcomers
>> alike!) the better.
>>
>> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
>>
>>
>>
>> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <
>> lewandowski.jacek@gmail.com> wrote:
>>
>> There is another target called "build", which retrieves dependencies, and
>> then calls "build-project".
>>
>>
>>
>> Is it intended to be called by a user ?
>>
>> If not, please follow the ant style prefixing the target name with an
>> underscore (so that it does not appear in the `ant -projecthelp` list).
>>
>> If possible, I agree with Brandon, `build` is the better name to expose
>> to the user.
>>
>>
>>
>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jeremiah Jordan <je...@gmail.com>]

 +100 I support making generate-idea-files auto setup everything in
IntelliJ for you.  If you post a diff, I will test it.

On this proposal, I don’t really have an opinion one way or the other about
what the default is for local "ant jar”, if its slow I will figure out how
to turn it off, if its fast I will leave it on.
I do care that CI runs checks, and complains loudly if something is wrong
such that it is very easy to tell during review.

-Jeremiah

On Jun 29, 2023 at 1:44:09 PM, Josh McKenzie <jm...@apache.org> wrote:

> In accord I added an opt-out for each hook, and will require such here as
> well
>
> On for main branches, off for feature branches seems like it might blanket
> satisfy this concern? Doesn't fix the "--atomic across 5 branches means
> style checks and build on hook across those branches" which isn't ideal. I
> don't think style check failures after push upstream are frequent enough to
> make the cost/benefit there make sense overall are they?
>
> Related to this - I have sonarlint, spotbugs, and checkstyle all running
> inside idea; since pulling those in and tuning the configs a bit I haven't
> run into a single issue w/our checkstyle build target (go figure). Having
> the required style checks reflected realtime inside your work environment
> goes a long way towards making it a more intuitive part of your workflow
> rather than being an annoying last minute block of your ability to progress
> that requires circling back into the code.
>
> From a technical perspective, it looks like adding a reference
> "externalDependencies.xml" to our ide/idea directory which we copied over
> during "generate-idea-files" would be sufficient to get idea to pop up
> prompts to install those extensions if you don't have them when opening the
> project (theory; haven't tested).
>
> We'd need to make sure the configuration for each of those was calibrated
> to our project out of the box of course, but making style considerations a
> first-class citizen in that way seems a more intuitive and human-centered
> approach to all this rather than debating nuance of our command-line
> targets, hooks, and how we present things to people. To Berenguer's point -
> better to have these be completely invisible to people with their workflows
> and Just Work (except for when your IDE scolds you for bad behavior w/build
> errors immediately).
>
> I still think Flags Are Bad. :)
>
> On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
>
> Should we just keep a consolidated for all kind of checks no-check flag
> and get rid of the no-checkstyle one?
>
> Trading one for one with Josh :-)
>
> Best regards,
> Ekaterina
>
> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org> wrote:
>
>
> I really prefer separate tasks than flags. Flags are not listed in the
> help message like "ant -p" and are not auto-completed in the terminal. That
> makes them almost undiscoverable for newcomers.
>
> Please, no more flags. We are *more* than flaggy enough right now.
>
> Having to dig through build.xml to determine how to change things or do
> things is painful; the more we can avoid this (for oldtimers and newcomers
> alike!) the better.
>
> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
>
>
>
> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <
> lewandowski.jacek@gmail.com> wrote:
>
> There is another target called "build", which retrieves dependencies, and
> then calls "build-project".
>
>
>
> Is it intended to be called by a user ?
>
> If not, please follow the ant style prefixing the target name with an
> underscore (so that it does not appear in the `ant -projecthelp` list).
>
> If possible, I agree with Brandon, `build` is the better name to expose to
> the user.
>
>
>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

["Miklosovic, Stefan" <St...@netapp.com>]

Great stuff, Josh! This is what I was talking about when I was mentioning that I am super curious about the workflows of other people. Any chance you share your setup somewhere so I may try it? Too soon to tell if we indeed want to go that direction but trying it out would be great ....

________________________________________
From: Josh McKenzie <jm...@apache.org>
Sent: Thursday, June 29, 2023 20:44
To: dev
Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations

NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.



In accord I added an opt-out for each hook, and will require such here as well
On for main branches, off for feature branches seems like it might blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches means style checks and build on hook across those branches" which isn't ideal. I don't think style check failures after push upstream are frequent enough to make the cost/benefit there make sense overall are they?

Related to this - I have sonarlint, spotbugs, and checkstyle all running inside idea; since pulling those in and tuning the configs a bit I haven't run into a single issue w/our checkstyle build target (go figure). Having the required style checks reflected realtime inside your work environment goes a long way towards making it a more intuitive part of your workflow rather than being an annoying last minute block of your ability to progress that requires circling back into the code.

From a technical perspective, it looks like adding a reference "externalDependencies.xml" to our ide/idea directory which we copied over during "generate-idea-files" would be sufficient to get idea to pop up prompts to install those extensions if you don't have them when opening the project (theory; haven't tested).

We'd need to make sure the configuration for each of those was calibrated to our project out of the box of course, but making style considerations a first-class citizen in that way seems a more intuitive and human-centered approach to all this rather than debating nuance of our command-line targets, hooks, and how we present things to people. To Berenguer's point - better to have these be completely invisible to people with their workflows and Just Work (except for when your IDE scolds you for bad behavior w/build errors immediately).

I still think Flags Are Bad. :)

On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
Should we just keep a consolidated for all kind of checks no-check flag and get rid of the no-checkstyle one?

Trading one for one with Josh :-)

Best regards,
Ekaterina

On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org>> wrote:

I really prefer separate tasks than flags. Flags are not listed in the help message like "ant -p" and are not auto-completed in the terminal. That makes them almost undiscoverable for newcomers.
Please, no more flags. We are more than flaggy enough right now.

Having to dig through build.xml to determine how to change things or do things is painful; the more we can avoid this (for oldtimers and newcomers alike!) the better.

On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:


On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <le...@gmail.com>> wrote:
There is another target called "build", which retrieves dependencies, and then calls "build-project".


Is it intended to be called by a user ?

If not, please follow the ant style prefixing the target name with an underscore (so that it does not appear in the `ant -projecthelp` list).

If possible, I agree with Brandon, `build` is the better name to expose to the user.

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Josh McKenzie <jm...@apache.org>]

> In accord I added an opt-out for each hook, and will require such here as well
On for main branches, off for feature branches seems like it might blanket satisfy this concern? Doesn't fix the "--atomic across 5 branches means style checks and build on hook across those branches" which isn't ideal. I don't think style check failures after push upstream are frequent enough to make the cost/benefit there make sense overall are they?

Related to this - I have sonarlint, spotbugs, and checkstyle all running inside idea; since pulling those in and tuning the configs a bit I haven't run into a single issue w/our checkstyle build target (go figure). Having the required style checks reflected realtime inside your work environment goes a long way towards making it a more intuitive part of your workflow rather than being an annoying last minute block of your ability to progress that requires circling back into the code.

From a technical perspective, it looks like adding a reference "externalDependencies.xml" to our ide/idea directory which we copied over during "generate-idea-files" would be sufficient to get idea to pop up prompts to install those extensions if you don't have them when opening the project (theory; haven't tested).

We'd need to make sure the configuration for each of those was calibrated to our project out of the box of course, but making style considerations a first-class citizen in that way seems a more intuitive and human-centered approach to all this rather than debating nuance of our command-line targets, hooks, and how we present things to people. To Berenguer's point - better to have these be completely invisible to people with their workflows and Just Work (except for when your IDE scolds you for bad behavior w/build errors immediately).

I still think Flags Are Bad. :)

On Thu, Jun 29, 2023, at 1:38 PM, Ekaterina Dimitrova wrote:
> Should we just keep a consolidated for all kind of checks no-check flag and get rid of the no-checkstyle one? 
> 
> Trading one for one with Josh :-) 
> 
> Best regards,
> Ekaterina
> 
> On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org> wrote:
>> __
>>> I really prefer separate tasks than flags. Flags are not listed in the help message like "ant -p" and are not auto-completed in the terminal. That makes them almost undiscoverable for newcomers. 
>> Please, no more flags. We are *more* than flaggy enough right now.
>> 
>> Having to dig through build.xml to determine how to change things or do things is painful; the more we can avoid this (for oldtimers and newcomers alike!) the better.
>> 
>> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
>>> 
>>> 
>>> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <le...@gmail.com> wrote:
>>>> There is another target called "build", which retrieves dependencies, and then calls "build-project".
>>> 
>>> 
>>> Is it intended to be called by a user ? 
>>> 
>>> If not, please follow the ant style prefixing the target name with an underscore (so that it does not appear in the `ant -projecthelp` list).
>>> 
>>> If possible, I agree with Brandon, `build` is the better name to expose to the user.
>> 

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Ekaterina Dimitrova <e....@gmail.com>]

Should we just keep a consolidated for all kind of checks no-check flag and
get rid of the no-checkstyle one?

Trading one for one with Josh :-)

Best regards,
Ekaterina

On Thu, 29 Jun 2023 at 10:52, Josh McKenzie <jm...@apache.org> wrote:

> I really prefer separate tasks than flags. Flags are not listed in the
> help message like "ant -p" and are not auto-completed in the terminal. That
> makes them almost undiscoverable for newcomers.
>
> Please, no more flags. We are *more* than flaggy enough right now.
>
> Having to dig through build.xml to determine how to change things or do
> things is painful; the more we can avoid this (for oldtimers and newcomers
> alike!) the better.
>
> On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
>
>
>
> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <
> lewandowski.jacek@gmail.com> wrote:
>
> There is another target called "build", which retrieves dependencies, and
> then calls "build-project".
>
>
>
> Is it intended to be called by a user ?
>
> If not, please follow the ant style prefixing the target name with an
> underscore (so that it does not appear in the `ant -projecthelp` list).
>
> If possible, I agree with Brandon, `build` is the better name to expose to
> the user.
>
>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Josh McKenzie <jm...@apache.org>]

> I really prefer separate tasks than flags. Flags are not listed in the help message like "ant -p" and are not auto-completed in the terminal. That makes them almost undiscoverable for newcomers. 
Please, no more flags. We are *more* than flaggy enough right now.

Having to dig through build.xml to determine how to change things or do things is painful; the more we can avoid this (for oldtimers and newcomers alike!) the better.

On Thu, Jun 29, 2023, at 8:34 AM, Mick Semb Wever wrote:
> 
> 
> On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <le...@gmail.com> wrote:
>> There is another target called "build", which retrieves dependencies, and then calls "build-project".
> 
> 
> Is it intended to be called by a user ? 
> 
> If not, please follow the ant style prefixing the target name with an underscore (so that it does not appear in the `ant -projecthelp` list).
> 
> If possible, I agree with Brandon, `build` is the better name to expose to the user.

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Mick Semb Wever <mc...@apache.org>]

On Thu, 29 Jun 2023 at 13:30, Jacek Lewandowski <le...@gmail.com>
wrote:

> There is another target called "build", which retrieves dependencies, and
> then calls "build-project".
>


Is it intended to be called by a user ?

If not, please follow the ant style prefixing the target name with an
underscore (so that it does not appear in the `ant -projecthelp` list).

If possible, I agree with Brandon, `build` is the better name to expose to
the user.

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

There is another target called "build", which retrieves dependencies, and
then calls "build-project".


czw., 29 cze 2023 o 12:33 Brandon Williams <dr...@gmail.com> napisał(a):

> This sounds good to me.  Can we shorten 'build-project' to just 'build'?
>
> Kind Regards,
> Brandon
>
> On Thu, Jun 29, 2023 at 3:22 AM Jacek Lewandowski
> <le...@gmail.com> wrote:
> >
> > So given all the feedback, I'm going to do the following:
> >
> > "jar" will depend on "check" target
> > "build-project", "build-test" and "test" targets will not depend on
> "check" target
> > "check" target will include checkstyle, rat and eclipse-warnings
> >
> > There is an additional flag "no-check" to disable checks in the "jar"
> target.
> >
> > Will not introduce any Git hook.
> >
> > wt., 27 cze 2023 o 18:35 Jacek Lewandowski <le...@gmail.com>
> napisał(a):
> >>
> >> With git you can always opt-out by adding --no-verify flag to either
> push or commit
> >>
> >> I really prefer separate tasks than flags. Flags are not listed in the
> help message like "ant -p" and are not auto-completed in the terminal. That
> makes them almost undiscoverable for newcomers.
> >>
> >> Want to have jar include checks? Ok, but let's don't run checks
> automatically with "build" or "test"
> >>
> >>
> >>
> >> wt., 27 cze 2023 o 18:26 David Capwell <dc...@apple.com> napisał(a):
> >>>
> >>> nobody referred to running checks in a pre-push (or pre-commit) hook
> >>>
> >>>
> >>>
> >>> In accord I added an opt-out for each hook, and will require such here
> as well… as long as you can opt-out, its fine by me… I know I will likely
> opt-out, but wouldn’t block such an effort
> >>>
> >>> Your point that pre-push hook might not be the best one is valid, and
> we should rather think about pre-commit
> >>>
> >>>
> >>> Pre-push is far better than pre-commit, with pre-commit you are
> forcing a style on people…. I for one have many many commits in a single
> PR, where I use commits to not loose track of progress (even if the code
> doesn’t compile), so forcing the build to work would be a -1 from me….
> Pre-push at least means “you want the world to see this” so makes more
> sense there…
> >>>
> >>> Again, must have an opt-out
> >>>
> >>> proposed:
> >>> ant jar (just build)
> >>> git commit (run some checks)
> >>>
> >>>
> >>> I am against this, jar should also check and ask users to opt-out if
> they don’t want the checks….
> >>>
> >>> If we go with opt-out i'd like to see one flag that can disable all
> checks: `-Dchecks.skip`
> >>>
> >>>
> >>> Works for me, you can also do the following to disable and not worry
> about
> >>>
> >>> $ cat <<EOF > build.properties
> >>> checks.skip: true
> >>> EOF
> >>>
> >>> On Jun 27, 2023, at 3:14 AM, Mick Semb Wever <mc...@apache.org> wrote:
> >>>
> >>> The context is that we currently have 3 checks in the build:
> >>>
> >>> - Checkstyle,
> >>> - Eclipse-Warnings,
> >>> - RAT
> >>>
> >>>
> >>>
> >>> And dependency-check (owasp).
> >>>
> >>>
> >>>
> >>> I want to discuss whether you are ok with extracting all checks to
> their distinct target and not running it automatically with the targets
> which devs usually run locally. In particular:
> >>>
> >>>
> >>> "build", "jar", and all "test" targets would not trigger CheckStyle,
> RAT or Eclipse-Warnings
> >>> A new target "check" would trigger all CheckStyle, RAT, and
> Eclipse-Warnings
> >>> The new "check" target would be run along with the "artifacts" target
> on Jenkins-CI, and it as a separate build step in CircleCI
> >>>
> >>>
> >>>
> >>> +0 I prefer this opt-in over an opt-out approach.
> >>>
> >>> It should be separated from `artifacts` too.
> >>> We would need to encourage engineers to run `ant check` before
> >>> starting CI and/or requesting review.
> >>>
> >>> I'm in favour of the opt-in approach because it keeps it visible.
> >>> Folks configure flags and it "disappears" forever.  Also it's a
> >>> headache in all the other ant targets where we actually don't want it,
> >>> e.g. tests.
> >>>
> >>> If we go with opt-out i'd like to see one flag that can disable all
> >>> checks: `-Dchecks.skip`
> >>>
> >>>
> >>> That could be fixed by running checks in a pre-push Git hook. There
> are some benefits of this compared to the current behavior:
> >>>
> >>>
> >>>
> >>> -1
> >>> committing and pushing to a personal branch is often done to save work
> >>> or for cross-machine or collaboration. We should not gate on checks or
> >>> compilation here.
> >>>
> >>> PRs should fail if checks fail, to give newcomers clear feedback (and
> >>> to take this nit-picking out of the review process).
> >>>
> >>>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Brandon Williams <dr...@gmail.com>]

This sounds good to me.  Can we shorten 'build-project' to just 'build'?

Kind Regards,
Brandon

On Thu, Jun 29, 2023 at 3:22 AM Jacek Lewandowski
<le...@gmail.com> wrote:
>
> So given all the feedback, I'm going to do the following:
>
> "jar" will depend on "check" target
> "build-project", "build-test" and "test" targets will not depend on "check" target
> "check" target will include checkstyle, rat and eclipse-warnings
>
> There is an additional flag "no-check" to disable checks in the "jar" target.
>
> Will not introduce any Git hook.
>
> wt., 27 cze 2023 o 18:35 Jacek Lewandowski <le...@gmail.com> napisał(a):
>>
>> With git you can always opt-out by adding --no-verify flag to either push or commit
>>
>> I really prefer separate tasks than flags. Flags are not listed in the help message like "ant -p" and are not auto-completed in the terminal. That makes them almost undiscoverable for newcomers.
>>
>> Want to have jar include checks? Ok, but let's don't run checks automatically with "build" or "test"
>>
>>
>>
>> wt., 27 cze 2023 o 18:26 David Capwell <dc...@apple.com> napisał(a):
>>>
>>> nobody referred to running checks in a pre-push (or pre-commit) hook
>>>
>>>
>>>
>>> In accord I added an opt-out for each hook, and will require such here as well… as long as you can opt-out, its fine by me… I know I will likely opt-out, but wouldn’t block such an effort
>>>
>>> Your point that pre-push hook might not be the best one is valid, and we should rather think about pre-commit
>>>
>>>
>>> Pre-push is far better than pre-commit, with pre-commit you are forcing a style on people…. I for one have many many commits in a single PR, where I use commits to not loose track of progress (even if the code doesn’t compile), so forcing the build to work would be a -1 from me…. Pre-push at least means “you want the world to see this” so makes more sense there…
>>>
>>> Again, must have an opt-out
>>>
>>> proposed:
>>> ant jar (just build)
>>> git commit (run some checks)
>>>
>>>
>>> I am against this, jar should also check and ask users to opt-out if they don’t want the checks….
>>>
>>> If we go with opt-out i'd like to see one flag that can disable all checks: `-Dchecks.skip`
>>>
>>>
>>> Works for me, you can also do the following to disable and not worry about
>>>
>>> $ cat <<EOF > build.properties
>>> checks.skip: true
>>> EOF
>>>
>>> On Jun 27, 2023, at 3:14 AM, Mick Semb Wever <mc...@apache.org> wrote:
>>>
>>> The context is that we currently have 3 checks in the build:
>>>
>>> - Checkstyle,
>>> - Eclipse-Warnings,
>>> - RAT
>>>
>>>
>>>
>>> And dependency-check (owasp).
>>>
>>>
>>>
>>> I want to discuss whether you are ok with extracting all checks to their distinct target and not running it automatically with the targets which devs usually run locally. In particular:
>>>
>>>
>>> "build", "jar", and all "test" targets would not trigger CheckStyle, RAT or Eclipse-Warnings
>>> A new target "check" would trigger all CheckStyle, RAT, and Eclipse-Warnings
>>> The new "check" target would be run along with the "artifacts" target on Jenkins-CI, and it as a separate build step in CircleCI
>>>
>>>
>>>
>>> +0 I prefer this opt-in over an opt-out approach.
>>>
>>> It should be separated from `artifacts` too.
>>> We would need to encourage engineers to run `ant check` before
>>> starting CI and/or requesting review.
>>>
>>> I'm in favour of the opt-in approach because it keeps it visible.
>>> Folks configure flags and it "disappears" forever.  Also it's a
>>> headache in all the other ant targets where we actually don't want it,
>>> e.g. tests.
>>>
>>> If we go with opt-out i'd like to see one flag that can disable all
>>> checks: `-Dchecks.skip`
>>>
>>>
>>> That could be fixed by running checks in a pre-push Git hook. There are some benefits of this compared to the current behavior:
>>>
>>>
>>>
>>> -1
>>> committing and pushing to a personal branch is often done to save work
>>> or for cross-machine or collaboration. We should not gate on checks or
>>> compilation here.
>>>
>>> PRs should fail if checks fail, to give newcomers clear feedback (and
>>> to take this nit-picking out of the review process).
>>>
>>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

So given all the feedback, I'm going to do the following:

"jar" will depend on "check" target
"build-project", "build-test" and "test" targets will not depend on "check"
target
"check" target will include checkstyle, rat and eclipse-warnings

There is an additional flag "no-check" to disable checks in the "jar"
target.

Will not introduce any Git hook.

wt., 27 cze 2023 o 18:35 Jacek Lewandowski <le...@gmail.com>
napisał(a):

> With git you can always opt-out by adding --no-verify flag to either push
> or commit
>
> I really prefer separate tasks than flags. Flags are not listed in the
> help message like "ant -p" and are not auto-completed in the terminal. That
> makes them almost undiscoverable for newcomers.
>
> Want to have jar include checks? Ok, but let's don't run checks
> automatically with "build" or "test"
>
>
>
> wt., 27 cze 2023 o 18:26 David Capwell <dc...@apple.com> napisał(a):
>
>> nobody referred to running checks in a pre-push (or pre-commit) hook
>>
>>
>>
>> In accord I added an opt-out for each hook, and will require such here as
>> well… as long as you can opt-out, its fine by me… I know I will likely
>> opt-out, but wouldn’t block such an effort
>>
>> Your point that pre-push hook might not be the best one is valid, and we
>> should rather think about pre-commit
>>
>>
>> Pre-push is far better than pre-commit, with pre-commit you are forcing a
>> style on people…. I for one have many many commits in a single PR, where I
>> use commits to not loose track of progress (even if the code doesn’t
>> compile), so forcing the build to work would be a -1 from me…. Pre-push at
>> least means “you want the world to see this” so makes more sense there…
>>
>> Again, must have an opt-out
>>
>> proposed:
>> ant jar (just build)
>> git commit (run some checks)
>>
>>
>> I am against this, jar should also check and ask users to opt-out if they
>> don’t want the checks….
>>
>> If we go with opt-out i'd like to see one flag that can disable all checks:
>> `-Dchecks.skip`
>>
>>
>> Works for me, you can also do the following to disable and not worry about
>>
>> $ cat <<EOF > build.properties
>> checks.skip: true
>> EOF
>>
>> On Jun 27, 2023, at 3:14 AM, Mick Semb Wever <mc...@apache.org> wrote:
>>
>> The context is that we currently have 3 checks in the build:
>>
>> - Checkstyle,
>> - Eclipse-Warnings,
>> - RAT
>>
>>
>>
>> And dependency-check (owasp).
>>
>>
>>
>> I want to discuss whether you are ok with extracting all checks to their
>> distinct target and not running it automatically with the targets which
>> devs usually run locally. In particular:
>>
>>
>> "build", "jar", and all "test" targets would not trigger CheckStyle, RAT
>> or Eclipse-Warnings
>> A new target "check" would trigger all CheckStyle, RAT, and
>> Eclipse-Warnings
>> The new "check" target would be run along with the "artifacts" target on
>> Jenkins-CI, and it as a separate build step in CircleCI
>>
>>
>>
>> +0 I prefer this opt-in over an opt-out approach.
>>
>> It should be separated from `artifacts` too.
>> We would need to encourage engineers to run `ant check` before
>> starting CI and/or requesting review.
>>
>> I'm in favour of the opt-in approach because it keeps it visible.
>> Folks configure flags and it "disappears" forever.  Also it's a
>> headache in all the other ant targets where we actually don't want it,
>> e.g. tests.
>>
>> If we go with opt-out i'd like to see one flag that can disable all
>> checks: `-Dchecks.skip`
>>
>>
>> That could be fixed by running checks in a pre-push Git hook. There are
>> some benefits of this compared to the current behavior:
>>
>>
>>
>> -1
>> committing and pushing to a personal branch is often done to save work
>> or for cross-machine or collaboration. We should not gate on checks or
>> compilation here.
>>
>> PRs should fail if checks fail, to give newcomers clear feedback (and
>> to take this nit-picking out of the review process).
>>
>>
>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

With git you can always opt-out by adding --no-verify flag to either push
or commit

I really prefer separate tasks than flags. Flags are not listed in the help
message like "ant -p" and are not auto-completed in the terminal. That
makes them almost undiscoverable for newcomers.

Want to have jar include checks? Ok, but let's don't run checks
automatically with "build" or "test"



wt., 27 cze 2023 o 18:26 David Capwell <dc...@apple.com> napisał(a):

> nobody referred to running checks in a pre-push (or pre-commit) hook
>
>
>
> In accord I added an opt-out for each hook, and will require such here as
> well… as long as you can opt-out, its fine by me… I know I will likely
> opt-out, but wouldn’t block such an effort
>
> Your point that pre-push hook might not be the best one is valid, and we
> should rather think about pre-commit
>
>
> Pre-push is far better than pre-commit, with pre-commit you are forcing a
> style on people…. I for one have many many commits in a single PR, where I
> use commits to not loose track of progress (even if the code doesn’t
> compile), so forcing the build to work would be a -1 from me…. Pre-push at
> least means “you want the world to see this” so makes more sense there…
>
> Again, must have an opt-out
>
> proposed:
> ant jar (just build)
> git commit (run some checks)
>
>
> I am against this, jar should also check and ask users to opt-out if they
> don’t want the checks….
>
> If we go with opt-out i'd like to see one flag that can disable all checks:
> `-Dchecks.skip`
>
>
> Works for me, you can also do the following to disable and not worry about
>
> $ cat <<EOF > build.properties
> checks.skip: true
> EOF
>
> On Jun 27, 2023, at 3:14 AM, Mick Semb Wever <mc...@apache.org> wrote:
>
> The context is that we currently have 3 checks in the build:
>
> - Checkstyle,
> - Eclipse-Warnings,
> - RAT
>
>
>
> And dependency-check (owasp).
>
>
>
> I want to discuss whether you are ok with extracting all checks to their
> distinct target and not running it automatically with the targets which
> devs usually run locally. In particular:
>
>
> "build", "jar", and all "test" targets would not trigger CheckStyle, RAT
> or Eclipse-Warnings
> A new target "check" would trigger all CheckStyle, RAT, and
> Eclipse-Warnings
> The new "check" target would be run along with the "artifacts" target on
> Jenkins-CI, and it as a separate build step in CircleCI
>
>
>
> +0 I prefer this opt-in over an opt-out approach.
>
> It should be separated from `artifacts` too.
> We would need to encourage engineers to run `ant check` before
> starting CI and/or requesting review.
>
> I'm in favour of the opt-in approach because it keeps it visible.
> Folks configure flags and it "disappears" forever.  Also it's a
> headache in all the other ant targets where we actually don't want it,
> e.g. tests.
>
> If we go with opt-out i'd like to see one flag that can disable all
> checks: `-Dchecks.skip`
>
>
> That could be fixed by running checks in a pre-push Git hook. There are
> some benefits of this compared to the current behavior:
>
>
>
> -1
> committing and pushing to a personal branch is often done to save work
> or for cross-machine or collaboration. We should not gate on checks or
> compilation here.
>
> PRs should fail if checks fail, to give newcomers clear feedback (and
> to take this nit-picking out of the review process).
>
>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[David Capwell <dc...@apple.com>]

> nobody referred to running checks in a pre-push (or pre-commit) hook


In accord I added an opt-out for each hook, and will require such here as well… as long as you can opt-out, its fine by me… I know I will likely opt-out, but wouldn’t block such an effort

> Your point that pre-push hook might not be the best one is valid, and we should rather think about pre-commit

Pre-push is far better than pre-commit, with pre-commit you are forcing a style on people…. I for one have many many commits in a single PR, where I use commits to not loose track of progress (even if the code doesn’t compile), so forcing the build to work would be a -1 from me…. Pre-push at least means “you want the world to see this” so makes more sense there…

Again, must have an opt-out

> proposed:
> ant jar (just build)
> git commit (run some checks)

I am against this, jar should also check and ask users to opt-out if they don’t want the checks….

> If we go with opt-out i'd like to see one flag that can disable all checks: `-Dchecks.skip`

Works for me, you can also do the following to disable and not worry about

$ cat <<EOF > build.properties
checks.skip: true
EOF

> On Jun 27, 2023, at 3:14 AM, Mick Semb Wever <mc...@apache.org> wrote:
> 
>> The context is that we currently have 3 checks in the build:
>> 
>> - Checkstyle,
>> - Eclipse-Warnings,
>> - RAT
> 
> 
> And dependency-check (owasp).
> 
> 
> 
>> I want to discuss whether you are ok with extracting all checks to their distinct target and not running it automatically with the targets which devs usually run locally. In particular:
>> 
>> 
>> "build", "jar", and all "test" targets would not trigger CheckStyle, RAT or Eclipse-Warnings
>> A new target "check" would trigger all CheckStyle, RAT, and Eclipse-Warnings
>> The new "check" target would be run along with the "artifacts" target on Jenkins-CI, and it as a separate build step in CircleCI
> 
> 
> +0 I prefer this opt-in over an opt-out approach.
> 
> It should be separated from `artifacts` too.
> We would need to encourage engineers to run `ant check` before
> starting CI and/or requesting review.
> 
> I'm in favour of the opt-in approach because it keeps it visible.
> Folks configure flags and it "disappears" forever.  Also it's a
> headache in all the other ant targets where we actually don't want it,
> e.g. tests.
> 
> If we go with opt-out i'd like to see one flag that can disable all
> checks: `-Dchecks.skip`
> 
> 
>> That could be fixed by running checks in a pre-push Git hook. There are some benefits of this compared to the current behavior:
> 
> 
> -1
> committing and pushing to a personal branch is often done to save work
> or for cross-machine or collaboration. We should not gate on checks or
> compilation here.
> 
> PRs should fail if checks fail, to give newcomers clear feedback (and
> to take this nit-picking out of the review process).

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Mick Semb Wever <mc...@apache.org>]

> The context is that we currently have 3 checks in the build:
>
> - Checkstyle,
> - Eclipse-Warnings,
> - RAT


And dependency-check (owasp).



> I want to discuss whether you are ok with extracting all checks to their distinct target and not running it automatically with the targets which devs usually run locally. In particular:
>
>
> "build", "jar", and all "test" targets would not trigger CheckStyle, RAT or Eclipse-Warnings
> A new target "check" would trigger all CheckStyle, RAT, and Eclipse-Warnings
> The new "check" target would be run along with the "artifacts" target on Jenkins-CI, and it as a separate build step in CircleCI


+0 I prefer this opt-in over an opt-out approach.

It should be separated from `artifacts` too.
We would need to encourage engineers to run `ant check` before
starting CI and/or requesting review.

I'm in favour of the opt-in approach because it keeps it visible.
Folks configure flags and it "disappears" forever.  Also it's a
headache in all the other ant targets where we actually don't want it,
e.g. tests.

If we go with opt-out i'd like to see one flag that can disable all
checks: `-Dchecks.skip`


> That could be fixed by running checks in a pre-push Git hook. There are some benefits of this compared to the current behavior:


-1
committing and pushing to a personal branch is often done to save work
or for cross-machine or collaboration. We should not gate on checks or
compilation here.

PRs should fail if checks fail, to give newcomers clear feedback (and
to take this nit-picking out of the review process).

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

Berenguer, as I said, I started this discussion because it is confusing
that we do implicit and unexpected tasks.
It is inconsistent that we run checkstyle, but we skip static code analysis
like Eclipse-Warnings because that actually falsifies the advantages of
running checks automatically.
More robust static code analysis will take even more time than
Eclipse-Warnings. Eventually, nobody is guaranteed to run any Ant task
before pushing if the whole development is done in IDE.

You basically want those tasks to be guaranteed to run locally before
pushing, which could be addressed more consistently by adding a Git hook.
Do you think we can encounter some particular problems with this approach?

Maxim, this is a great idea, and I fully support it - but this does not
address the issues I've raised there.
- - -- --- ----- -------- -------------
Jacek Lewandowski


pon., 26 cze 2023 o 14:05 Maxim Muzafarov <mm...@apache.org> napisał(a):

> Hello everyone,
>
> We can replace RAT with the appropriate checkstyle rule - the HeaderCheck,
> I think. This will reduce the number of tools we now use and reduce the
> build time as only modified files will be checked, and this, in turn, will
> remove some of the concerns mentioned in the first message.
>
> https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheck.html
>
>
>
> On Mon, 26 Jun 2023 at 13:48, Berenguer Blasi <be...@gmail.com>
> wrote:
>
>> Just for awareness if you rebase thanks to CASSANDRA-18588 checkstyle
>> shouldn't be a problem anymore. If it is still let me know and I can look
>> into it.
>> On 26/6/23 13:11, Jacek Lewandowski wrote:
>>
>> Yes, I've mentioned that there is a property we can set to skip
>> checkstyle.
>>
>> Currently such a goal is "artifacts" which basically validates everything.
>>
>>
>> - - -- --- ----- -------- -------------
>> Jacek Lewandowski
>>
>>
>> pon., 26 cze 2023 o 13:09 Mike Adamson <ma...@datastax.com>
>> napisał(a):
>>
>>> While I like the idea of this because of added time these checks take, I
>>> was under the impression that checkstyle (at least) can be disabled with a
>>> flag.
>>>
>>> If we did do this, would it make sense to have a "release"  or "commit"
>>> target (or some other name) that ran a full build with all checks that can
>>> be used prior to pushing changes?
>>>
>>> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <be...@gmail.com>
>>> wrote:
>>>
>>>> I would prefer sthg that is totally transparent to me and not add one
>>>> more step I have to remember. Just to push/run CI to find out I missed it
>>>> and rinse and repeat... With the recent fix to checkstyle I am happy as
>>>> things stand atm. My 2cts
>>>> On 26/6/23 8:43, Jacek Lewandowski wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> The context is that we currently have 3 checks in the build:
>>>>
>>>> - Checkstyle,
>>>>
>>>> - Eclipse-Warnings,
>>>>
>>>> - RAT
>>>>
>>>>
>>>> CheckStyle and RAT are executed with almost every target we run: build,
>>>> jar, test, test-some, testclasslist, etc.; on the other hand,
>>>> Eclipse-Warnings is executed automatically only with the artifacts target.
>>>>
>>>>
>>>> Checkstyle currently uses some caching, so subsequent reruns without
>>>> cleaning the project validate only the modified files.
>>>>
>>>>
>>>> Both CI - Jenkins and Circle forces running all checks.
>>>>
>>>>
>>>> I want to discuss whether you are ok with extracting all checks to
>>>> their distinct target and not running it automatically with the targets
>>>> which devs usually run locally. In particular:
>>>>
>>>>
>>>>
>>>>    - "build", "jar", and all "test" targets would not trigger
>>>>    CheckStyle, RAT or Eclipse-Warnings
>>>>    - A new target "check" would trigger all CheckStyle, RAT, and
>>>>    Eclipse-Warnings
>>>>    - The new "check" target would be run along with the "artifacts"
>>>>    target on Jenkins-CI, and it as a separate build step in CircleCI
>>>>
>>>>
>>>> The rationale for that change is:
>>>>
>>>>    - Running all the checks together would be more consistent, but
>>>>    running all of them automatically with build and test targets could waste
>>>>    time when we develop something locally, frequently rebuilding and running
>>>>    tests.
>>>>    - On the other hand, it would be more consistent if the build did
>>>>    what we want - as a dev, when prototyping, I don't want to be forced to run
>>>>    analysis (and potentially fix issues) whenever I want to build a project or
>>>>    just run a single test.
>>>>    - There are ways to avoid running checks automatically by
>>>>    specifying some build properties. Though, the discussion is about the
>>>>    default behavior - on the flip side, if one wants to run the checks along
>>>>    with the specified target, they could add the "check" target to the command
>>>>    line.
>>>>
>>>>
>>>> The rationale for keeping the checks running automatically with every
>>>> target is to reduce the likelihood of not running the checks locally before
>>>> pushing the branch and being surprised by failing CI soon after starting
>>>> the build.
>>>>
>>>>
>>>> That could be fixed by running checks in a pre-push Git hook. There are
>>>> some benefits of this compared to the current behavior:
>>>>
>>>>    - the checks would be run automatically only once
>>>>    - they would be triggered even for those devs who do everything in
>>>>    IDE and do not even touch Ant commands directly
>>>>
>>>>
>>>> Checks can take time; to optimize that, they could be enforced locally
>>>> to verify only the modified files in the same way as we currently determine
>>>> the tests to be repeated for CircleCI.
>>>>
>>>> Thanks
>>>> - - -- --- ----- -------- -------------
>>>> Jacek Lewandowski
>>>>
>>>>
>>>
>>> --
>>> [image: DataStax Logo Square] <https://www.datastax.com/> *Mike Adamson*
>>> Engineering
>>>
>>> +1 650 389 6000 <16503896000> | datastax.com <https://www.datastax.com/>
>>> Find DataStax Online: [image: LinkedIn Logo]
>>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
>>>    [image: Facebook Logo]
>>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
>>>    [image: Twitter Logo] <https://twitter.com/DataStax>   [image: RSS
>>> Feed] <https://www.datastax.com/blog/rss.xml>   [image: Github Logo]
>>> <https://github.com/datastax>
>>>
>>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Maxim Muzafarov <mm...@apache.org>]

Hello everyone,

We can replace RAT with the appropriate checkstyle rule - the HeaderCheck,
I think. This will reduce the number of tools we now use and reduce the
build time as only modified files will be checked, and this, in turn, will
remove some of the concerns mentioned in the first message.
https://checkstyle.org/apidocs/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheck.html



On Mon, 26 Jun 2023 at 13:48, Berenguer Blasi <be...@gmail.com>
wrote:

> Just for awareness if you rebase thanks to CASSANDRA-18588 checkstyle
> shouldn't be a problem anymore. If it is still let me know and I can look
> into it.
> On 26/6/23 13:11, Jacek Lewandowski wrote:
>
> Yes, I've mentioned that there is a property we can set to skip checkstyle.
>
> Currently such a goal is "artifacts" which basically validates everything.
>
>
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> pon., 26 cze 2023 o 13:09 Mike Adamson <ma...@datastax.com> napisał(a):
>
>> While I like the idea of this because of added time these checks take, I
>> was under the impression that checkstyle (at least) can be disabled with a
>> flag.
>>
>> If we did do this, would it make sense to have a "release"  or "commit"
>> target (or some other name) that ran a full build with all checks that can
>> be used prior to pushing changes?
>>
>> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <be...@gmail.com>
>> wrote:
>>
>>> I would prefer sthg that is totally transparent to me and not add one
>>> more step I have to remember. Just to push/run CI to find out I missed it
>>> and rinse and repeat... With the recent fix to checkstyle I am happy as
>>> things stand atm. My 2cts
>>> On 26/6/23 8:43, Jacek Lewandowski wrote:
>>>
>>> Hi,
>>>
>>>
>>> The context is that we currently have 3 checks in the build:
>>>
>>> - Checkstyle,
>>>
>>> - Eclipse-Warnings,
>>>
>>> - RAT
>>>
>>>
>>> CheckStyle and RAT are executed with almost every target we run: build,
>>> jar, test, test-some, testclasslist, etc.; on the other hand,
>>> Eclipse-Warnings is executed automatically only with the artifacts target.
>>>
>>>
>>> Checkstyle currently uses some caching, so subsequent reruns without
>>> cleaning the project validate only the modified files.
>>>
>>>
>>> Both CI - Jenkins and Circle forces running all checks.
>>>
>>>
>>> I want to discuss whether you are ok with extracting all checks to their
>>> distinct target and not running it automatically with the targets which
>>> devs usually run locally. In particular:
>>>
>>>
>>>
>>>    - "build", "jar", and all "test" targets would not trigger
>>>    CheckStyle, RAT or Eclipse-Warnings
>>>    - A new target "check" would trigger all CheckStyle, RAT, and
>>>    Eclipse-Warnings
>>>    - The new "check" target would be run along with the "artifacts"
>>>    target on Jenkins-CI, and it as a separate build step in CircleCI
>>>
>>>
>>> The rationale for that change is:
>>>
>>>    - Running all the checks together would be more consistent, but
>>>    running all of them automatically with build and test targets could waste
>>>    time when we develop something locally, frequently rebuilding and running
>>>    tests.
>>>    - On the other hand, it would be more consistent if the build did
>>>    what we want - as a dev, when prototyping, I don't want to be forced to run
>>>    analysis (and potentially fix issues) whenever I want to build a project or
>>>    just run a single test.
>>>    - There are ways to avoid running checks automatically by specifying
>>>    some build properties. Though, the discussion is about the default behavior
>>>    - on the flip side, if one wants to run the checks along with the specified
>>>    target, they could add the "check" target to the command line.
>>>
>>>
>>> The rationale for keeping the checks running automatically with every
>>> target is to reduce the likelihood of not running the checks locally before
>>> pushing the branch and being surprised by failing CI soon after starting
>>> the build.
>>>
>>>
>>> That could be fixed by running checks in a pre-push Git hook. There are
>>> some benefits of this compared to the current behavior:
>>>
>>>    - the checks would be run automatically only once
>>>    - they would be triggered even for those devs who do everything in
>>>    IDE and do not even touch Ant commands directly
>>>
>>>
>>> Checks can take time; to optimize that, they could be enforced locally
>>> to verify only the modified files in the same way as we currently determine
>>> the tests to be repeated for CircleCI.
>>>
>>> Thanks
>>> - - -- --- ----- -------- -------------
>>> Jacek Lewandowski
>>>
>>>
>>
>> --
>> [image: DataStax Logo Square] <https://www.datastax.com/> *Mike Adamson*
>> Engineering
>>
>> +1 650 389 6000 <16503896000> | datastax.com <https://www.datastax.com/>
>> Find DataStax Online: [image: LinkedIn Logo]
>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
>>    [image: Facebook Logo]
>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
>>    [image: Twitter Logo] <https://twitter.com/DataStax>   [image: RSS
>> Feed] <https://www.datastax.com/blog/rss.xml>   [image: Github Logo]
>> <https://github.com/datastax>
>>
>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Berenguer Blasi <be...@gmail.com>]

Just for awareness if you rebase thanks to CASSANDRA-18588 checkstyle 
shouldn't be a problem anymore. If it is still let me know and I can 
look into it.

On 26/6/23 13:11, Jacek Lewandowski wrote:
> Yes, I've mentioned that there is a property we can set to skip 
> checkstyle.
>
> Currently such a goal is "artifacts" which basically validates everything.
>
>
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> pon., 26 cze 2023 o 13:09 Mike Adamson <ma...@datastax.com> napisał(a):
>
>     While I like the idea of this because of added time these checks
>     take, I was under the impression that checkstyle (at least) can be
>     disabled with a flag.
>
>     If we did do this, would it make sense to have a "release"  or
>     "commit" target (or some other name) that ran a full build with
>     all checks that can be used prior to pushing changes?
>
>     On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi
>     <be...@gmail.com> wrote:
>
>         I would prefer sthg that is totally transparent to me and not
>         add one more step I have to remember. Just to push/run CI to
>         find out I missed it and rinse and repeat... With the recent
>         fix to checkstyle I am happy as things stand atm. My 2cts
>
>         On 26/6/23 8:43, Jacek Lewandowski wrote:
>>
>>         Hi,
>>
>>
>>         The context is that we currently have 3 checks in the build:
>>
>>         - Checkstyle,
>>
>>         - Eclipse-Warnings,
>>
>>         - RAT
>>
>>
>>         CheckStyle and RAT are executed with almost every target we
>>         run: build, jar, test, test-some, testclasslist, etc.; on the
>>         other hand, Eclipse-Warnings is executed automatically only
>>         with the artifacts target.
>>
>>
>>         Checkstyle currently uses some caching, so subsequent reruns
>>         without cleaning the project validate only the modified files.
>>
>>
>>         Both CI - Jenkins and Circle forces running all checks.
>>
>>
>>         I want to discuss whether you are ok with extracting all
>>         checks to their distinct target and not running it
>>         automatically with the targets which devs usually run
>>         locally. In particular:
>>
>>
>>           * "build", "jar", and all "test" targets would not trigger
>>             CheckStyle, RAT or Eclipse-Warnings
>>           * A new target "check" would trigger all CheckStyle, RAT,
>>             and Eclipse-Warnings
>>           * The new "check" target would be run along with the
>>             "artifacts" target on Jenkins-CI, and it as a separate
>>             build step in CircleCI
>>
>>
>>         The rationale for that change is:
>>
>>           * Running all the checks together would be more consistent,
>>             but running all of them automatically with build and test
>>             targets could waste time when we develop something
>>             locally, frequently rebuilding and running tests.
>>           * On the other hand, it would be more consistent if the
>>             build did what we want - as a dev, when prototyping, I
>>             don't want to be forced to run analysis (and potentially
>>             fix issues) whenever I want to build a project or just
>>             run a single test.
>>           * There are ways to avoid running checks automatically by
>>             specifying some build properties. Though, the discussion
>>             is about the default behavior - on the flip side, if one
>>             wants to run the checks along with the specified target,
>>             they could add the "check" target to the command line.
>>
>>
>>         The rationale for keeping the checks running automatically
>>         with every target is to reduce the likelihood of not running
>>         the checks locally before pushing the branch and being
>>         surprised by failing CI soon after starting the build.
>>
>>
>>         That could be fixed by running checks in a pre-push Git hook.
>>         There are some benefits of this compared to the current behavior:
>>
>>           * the checks would be run automatically only once
>>           * they would be triggered even for those devs who do
>>             everything in IDE and do not even touch Ant commands directly
>>
>>
>>         Checks can take time; to optimize that, they could be
>>         enforced locally to verify only the modified files in the
>>         same way as we currently determine the tests to be repeated
>>         for CircleCI.
>>
>>
>>         Thanks
>>         - - -- --- ----- -------- -------------
>>         Jacek Lewandowski
>
>
>
>     -- 
>     DataStax Logo Square <https://www.datastax.com/> 	*Mike Adamson*
>     Engineering
>
>     +1 650 389 6000 <tel:16503896000>|datastax.com
>     <https://www.datastax.com/>
>
>     Find DataStax Online: 	LinkedIn Logo
>     <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
>     Facebook Logo
>     <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
>     Twitter Logo <https://twitter.com/DataStax> RSS Feed
>     <https://www.datastax.com/blog/rss.xml> Github Logo
>     <https://github.com/datastax>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

Stefan, if you build using command line and then commit / push also in the
terminal, nothing would change for you:

now:
ant jar (automatically runs some checks)
git commit
git push

proposed:
ant jar (just build)
git commit (run some checks)
git push

Your point that pre-push hook might not be the best one is valid, and we
should rather think about pre-commit



- - -- --- ----- -------- -------------
Jacek Lewandowski


wt., 27 cze 2023 o 09:25 Miklosovic, Stefan <St...@netapp.com>
napisał(a):

> I am doing all git-related operations in the console / bash (IDEA
> terminal, alt+f12 in IDEA). I know IDEA can do git stuff as well but I
> never tried it and I just do not care. I just do not "believe it" (yeah,
> call me old-fashioned if you want) so for me how it looks like in IDEA
> around some checkboxes I have to turn off is irrelevant.
>
> I do not like the idea of git hooks. Maybe it is a matter of a strong
> habit but I am executing all these checks before I push anyway so for me
> the git hooks are not important and I would have to unlearn building it if
> git hook is going to do that for me instead.
>
> If I am going to push 5 branches like this:
>
> git push upstream cassandra-3.0 cassandra-3.11 cassandra-4.0 cassandra-4.1
> trunk --atomic
>
> This means that git hooks would start to build 5 branches again? What if
> somebody pushes as I am building it? Building 5 branches from scratch would
> take like 10 minutes, probably ...
>
> ________________________________________
> From: Jacek Lewandowski <le...@gmail.com>
> Sent: Tuesday, June 27, 2023 9:08
> To: dev@cassandra.apache.org
> Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations
>
> NetApp Security WARNING: This is an external email. Do not click links or
> open attachments unless you recognize the sender and know the content is
> safe.
>
>
>
> So far, nobody referred to running checks in a pre-push (or pre-commit)
> hook. The use of Git hooks is going to be introduced along with Accord, so
> we could use them to force running checks once before sending changes to
> the repo.
> It would still be an opt-out approach because one would have to add the
> "--no-verify" flag or uncheck a box in the commit dialog to skip running
> the checks.
>
> thanks,
> Jacek
>
>
> wt., 27 cze 2023 o 01:55 Ekaterina Dimitrova <e.dimitrova@gmail.com
> <ma...@gmail.com>> napisał(a):
> Thank you, Jacek, for starting the thread; those things are essential for
> developer productivity.
>
> I support the idea of opting out vs opting into checks. In my experience,
> it also makes things easier and faster during review time.
>
> If people have to opt-in - it is one more thing for new people to
> discover, and it will probably happen only during review time if they do
> not have access to Jenkins/paid CircleCI, etc.
>
> I also support consolidating all types of checks/analyses and running them
> together.
>
> Maxim’s suggestion about rat replacement sounds like a good improvement
> that can be explored (not part of what Jacek does here, though). Maxim, do
> you mind creating a ticket, please?
>
> Best regards,
> Ekaterina
>
> On Mon, 26 Jun 2023 at 17:04, Miklosovic, Stefan <
> Stefan.Miklosovic@netapp.com<ma...@netapp.com>> wrote:
> Yes, in this case, opting-out is better than opting-in. I feel like the
> build process is quite versatile and one just picks what is necessary. I
> never build docs, there is a flag for that. I turned off checkstyle because
> I was fed up with that until Berenguer cached it and now I get ant jar with
> checkstyle like under 10 seconds so I leave it on, which is great.
>
> Even though I feel like it is already flexible enough, grouping all
> checkstyles and rats etc under one target seems like a good idea. From my
> perspective, it is "all or nothing" so turning it all off until I am going
> to push it so I want it all on is a good idea. I barely want to "just
> checkstyle" in the middle of the development.
>
> I do not think that having a lot of flags is bad. I like that I have bash
> aliases almost for everything and I bet folks have their tricks to get the
> mundane stuff done.
>
> It would be pretty interesting to know the workflow of other people. I
> think there would be a lot of insights how other people have it on a daily
> basis when it comes to Cassandra development.
>
> ________________________________________
> From: David Capwell <dc...@apple.com>>
> Sent: Monday, June 26, 2023 19:57
> To: dev
> Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations
>
> NetApp Security WARNING: This is an external email. Do not click links or
> open attachments unless you recognize the sender and know the content is
> safe.
>
>
>
> not running it automatically with the targets which devs usually run
> locally.
>
> The checks tend to have an opt-out, such as -Dno-checkstyle=true… so its
> really easy to setup your local environment to opt out what you do not care
> about… I feel we should force people to opt-out rather than opt-in…
>
>
>
> On Jun 26, 2023, at 7:47 AM, Jacek Lewandowski <
> lewandowski.jacek@gmail.com<ma...@gmail.com>> wrote:
>
> That would work as well Brandon, basically what is proposed in
> CASSANDRA-18618, that is "check" target, actually needs to build the
> project to perform some verifications - I suppose running "ant check"
> should be sufficient.
>
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> pon., 26 cze 2023 o 16:01 Brandon Williams <driftx@gmail.com<mailto:
> driftx@gmail.com><ma...@gmail.com>>>
> napisał(a):
> The "artifacts" task is not quite the same since it builds other things
> like docs, which significantly contributes to longer build time.  I don't
> see why we couldn't add a new task that preserves the old behavior though,
> "fulljar" or something like that.
>
> Kind Regards,
> Brandon
>
>
> On Mon, Jun 26, 2023 at 6:12 AM Jacek Lewandowski <
> lewandowski.jacek@gmail.com<ma...@gmail.com><mailto:
> lewandowski.jacek@gmail.com<ma...@gmail.com>>> wrote:
> Yes, I've mentioned that there is a property we can set to skip checkstyle.
>
> Currently such a goal is "artifacts" which basically validates everything.
>
>
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> pon., 26 cze 2023 o 13:09 Mike Adamson <madamson@datastax.com<mailto:
> madamson@datastax.com><mailto:madamson@datastax.com<mailto:
> madamson@datastax.com>>> napisał(a):
> While I like the idea of this because of added time these checks take, I
> was under the impression that checkstyle (at least) can be disabled with a
> flag.
>
> If we did do this, would it make sense to have a "release"  or "commit"
> target (or some other name) that ran a full build with all checks that can
> be used prior to pushing changes?
>
> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <berenguerblasi@gmail.com
> <ma...@gmail.com><mailto:berenguerblasi@gmail.com<mailto:
> berenguerblasi@gmail.com>>> wrote:
>
> I would prefer sthg that is totally transparent to me and not add one more
> step I have to remember. Just to push/run CI to find out I missed it and
> rinse and repeat... With the recent fix to checkstyle I am happy as things
> stand atm. My 2cts
>
> On 26/6/23 8:43, Jacek Lewandowski wrote:
> Hi,
>
> The context is that we currently have 3 checks in the build:
> - Checkstyle,
> - Eclipse-Warnings,
> - RAT
>
> CheckStyle and RAT are executed with almost every target we run: build,
> jar, test, test-some, testclasslist, etc.; on the other hand,
> Eclipse-Warnings is executed automatically only with the artifacts target.
>
> Checkstyle currently uses some caching, so subsequent reruns without
> cleaning the project validate only the modified files.
>
> Both CI - Jenkins and Circle forces running all checks.
>
> I want to discuss whether you are ok with extracting all checks to their
> distinct target and not running it automatically with the targets which
> devs usually run locally. In particular:
>
>
>   *   "build", "jar", and all "test" targets would not trigger CheckStyle,
> RAT or Eclipse-Warnings
>   *   A new target "check" would trigger all CheckStyle, RAT, and
> Eclipse-Warnings
>   *   The new "check" target would be run along with the "artifacts"
> target on Jenkins-CI, and it as a separate build step in CircleCI
>
> The rationale for that change is:
>
>   *   Running all the checks together would be more consistent, but
> running all of them automatically with build and test targets could waste
> time when we develop something locally, frequently rebuilding and running
> tests.
>   *   On the other hand, it would be more consistent if the build did what
> we want - as a dev, when prototyping, I don't want to be forced to run
> analysis (and potentially fix issues) whenever I want to build a project or
> just run a single test.
>   *   There are ways to avoid running checks automatically by specifying
> some build properties. Though, the discussion is about the default behavior
> - on the flip side, if one wants to run the checks along with the specified
> target, they could add the "check" target to the command line.
>
> The rationale for keeping the checks running automatically with every
> target is to reduce the likelihood of not running the checks locally before
> pushing the branch and being surprised by failing CI soon after starting
> the build.
>
> That could be fixed by running checks in a pre-push Git hook. There are
> some benefits of this compared to the current behavior:
>
>   *   the checks would be run automatically only once
>   *   they would be triggered even for those devs who do everything in IDE
> and do not even touch Ant commands directly
>
> Checks can take time; to optimize that, they could be enforced locally to
> verify only the modified files in the same way as we currently determine
> the tests to be repeated for CircleCI.
>
> Thanks
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> --
> [DataStax Logo Square]<https://www.datastax.com/<https://www.datastax.com/>>
>    Mike Adamson
> Engineering
>
> +1 650 389 6000<tel:16503896000> | datastax.com<http://datastax.com/><
> https://www.datastax.com/<https://www.datastax.com/>>
> Find DataStax Online:   [LinkedIn Logo] <
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=
> <
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>>
>   [Facebook Logo] <
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=
> <
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>>
>   [Twitter Logo] <https://twitter.com/DataStax<
> https://twitter.com/DataStax>>    [RSS Feed] <
> https://www.datastax.com/blog/rss.xml<
> https://www.datastax.com/blog/rss.xml>>    [Github Logo] <
> https://github.com/datastax<https://github.com/datastax>>
>
>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

["Miklosovic, Stefan" <St...@netapp.com>]

I am doing all git-related operations in the console / bash (IDEA terminal, alt+f12 in IDEA). I know IDEA can do git stuff as well but I never tried it and I just do not care. I just do not "believe it" (yeah, call me old-fashioned if you want) so for me how it looks like in IDEA around some checkboxes I have to turn off is irrelevant.

I do not like the idea of git hooks. Maybe it is a matter of a strong habit but I am executing all these checks before I push anyway so for me the git hooks are not important and I would have to unlearn building it if git hook is going to do that for me instead.

If I am going to push 5 branches like this:

git push upstream cassandra-3.0 cassandra-3.11 cassandra-4.0 cassandra-4.1 trunk --atomic

This means that git hooks would start to build 5 branches again? What if somebody pushes as I am building it? Building 5 branches from scratch would take like 10 minutes, probably ...

________________________________________
From: Jacek Lewandowski <le...@gmail.com>
Sent: Tuesday, June 27, 2023 9:08
To: dev@cassandra.apache.org
Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations

NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.



So far, nobody referred to running checks in a pre-push (or pre-commit) hook. The use of Git hooks is going to be introduced along with Accord, so we could use them to force running checks once before sending changes to the repo.
It would still be an opt-out approach because one would have to add the "--no-verify" flag or uncheck a box in the commit dialog to skip running the checks.

thanks,
Jacek


wt., 27 cze 2023 o 01:55 Ekaterina Dimitrova <e....@gmail.com>> napisał(a):
Thank you, Jacek, for starting the thread; those things are essential for developer productivity.

I support the idea of opting out vs opting into checks. In my experience, it also makes things easier and faster during review time.

If people have to opt-in - it is one more thing for new people to discover, and it will probably happen only during review time if they do not have access to Jenkins/paid CircleCI, etc.

I also support consolidating all types of checks/analyses and running them together.

Maxim’s suggestion about rat replacement sounds like a good improvement that can be explored (not part of what Jacek does here, though). Maxim, do you mind creating a ticket, please?

Best regards,
Ekaterina

On Mon, 26 Jun 2023 at 17:04, Miklosovic, Stefan <St...@netapp.com>> wrote:
Yes, in this case, opting-out is better than opting-in. I feel like the build process is quite versatile and one just picks what is necessary. I never build docs, there is a flag for that. I turned off checkstyle because I was fed up with that until Berenguer cached it and now I get ant jar with checkstyle like under 10 seconds so I leave it on, which is great.

Even though I feel like it is already flexible enough, grouping all checkstyles and rats etc under one target seems like a good idea. From my perspective, it is "all or nothing" so turning it all off until I am going to push it so I want it all on is a good idea. I barely want to "just checkstyle" in the middle of the development.

I do not think that having a lot of flags is bad. I like that I have bash aliases almost for everything and I bet folks have their tricks to get the mundane stuff done.

It would be pretty interesting to know the workflow of other people. I think there would be a lot of insights how other people have it on a daily basis when it comes to Cassandra development.

________________________________________
From: David Capwell <dc...@apple.com>>
Sent: Monday, June 26, 2023 19:57
To: dev
Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations

NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.



not running it automatically with the targets which devs usually run locally.

The checks tend to have an opt-out, such as -Dno-checkstyle=true… so its really easy to setup your local environment to opt out what you do not care about… I feel we should force people to opt-out rather than opt-in…



On Jun 26, 2023, at 7:47 AM, Jacek Lewandowski <le...@gmail.com>> wrote:

That would work as well Brandon, basically what is proposed in CASSANDRA-18618, that is "check" target, actually needs to build the project to perform some verifications - I suppose running "ant check" should be sufficient.

- - -- --- ----- -------- -------------
Jacek Lewandowski


pon., 26 cze 2023 o 16:01 Brandon Williams <dr...@gmail.com>>> napisał(a):
The "artifacts" task is not quite the same since it builds other things like docs, which significantly contributes to longer build time.  I don't see why we couldn't add a new task that preserves the old behavior though, "fulljar" or something like that.

Kind Regards,
Brandon


On Mon, Jun 26, 2023 at 6:12 AM Jacek Lewandowski <le...@gmail.com>>> wrote:
Yes, I've mentioned that there is a property we can set to skip checkstyle.

Currently such a goal is "artifacts" which basically validates everything.


- - -- --- ----- -------- -------------
Jacek Lewandowski


pon., 26 cze 2023 o 13:09 Mike Adamson <ma...@datastax.com>>> napisał(a):
While I like the idea of this because of added time these checks take, I was under the impression that checkstyle (at least) can be disabled with a flag.

If we did do this, would it make sense to have a "release"  or "commit" target (or some other name) that ran a full build with all checks that can be used prior to pushing changes?

On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <be...@gmail.com>>> wrote:

I would prefer sthg that is totally transparent to me and not add one more step I have to remember. Just to push/run CI to find out I missed it and rinse and repeat... With the recent fix to checkstyle I am happy as things stand atm. My 2cts

On 26/6/23 8:43, Jacek Lewandowski wrote:
Hi,

The context is that we currently have 3 checks in the build:
- Checkstyle,
- Eclipse-Warnings,
- RAT

CheckStyle and RAT are executed with almost every target we run: build, jar, test, test-some, testclasslist, etc.; on the other hand, Eclipse-Warnings is executed automatically only with the artifacts target.

Checkstyle currently uses some caching, so subsequent reruns without cleaning the project validate only the modified files.

Both CI - Jenkins and Circle forces running all checks.

I want to discuss whether you are ok with extracting all checks to their distinct target and not running it automatically with the targets which devs usually run locally. In particular:


  *   "build", "jar", and all "test" targets would not trigger CheckStyle, RAT or Eclipse-Warnings
  *   A new target "check" would trigger all CheckStyle, RAT, and Eclipse-Warnings
  *   The new "check" target would be run along with the "artifacts" target on Jenkins-CI, and it as a separate build step in CircleCI

The rationale for that change is:

  *   Running all the checks together would be more consistent, but running all of them automatically with build and test targets could waste time when we develop something locally, frequently rebuilding and running tests.
  *   On the other hand, it would be more consistent if the build did what we want - as a dev, when prototyping, I don't want to be forced to run analysis (and potentially fix issues) whenever I want to build a project or just run a single test.
  *   There are ways to avoid running checks automatically by specifying some build properties. Though, the discussion is about the default behavior - on the flip side, if one wants to run the checks along with the specified target, they could add the "check" target to the command line.

The rationale for keeping the checks running automatically with every target is to reduce the likelihood of not running the checks locally before pushing the branch and being surprised by failing CI soon after starting the build.

That could be fixed by running checks in a pre-push Git hook. There are some benefits of this compared to the current behavior:

  *   the checks would be run automatically only once
  *   they would be triggered even for those devs who do everything in IDE and do not even touch Ant commands directly

Checks can take time; to optimize that, they could be enforced locally to verify only the modified files in the same way as we currently determine the tests to be repeated for CircleCI.

Thanks
- - -- --- ----- -------- -------------
Jacek Lewandowski


--
[DataStax Logo Square]<https://www.datastax.com/<https://www.datastax.com/>>     Mike Adamson
Engineering

+1 650 389 6000<tel:16503896000> | datastax.com<http://datastax.com/><https://www.datastax.com/<https://www.datastax.com/>>
Find DataStax Online:   [LinkedIn Logo] <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=<https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>>    [Facebook Logo] <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=<https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>>    [Twitter Logo] <https://twitter.com/DataStax<https://twitter.com/DataStax>>    [RSS Feed] <https://www.datastax.com/blog/rss.xml<https://www.datastax.com/blog/rss.xml>>    [Github Logo] <https://github.com/datastax<https://github.com/datastax>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

So far, nobody referred to running checks in a pre-push (or pre-commit)
hook. The use of Git hooks is going to be introduced along with Accord, so
we could use them to force running checks once before sending changes to
the repo.
It would still be an opt-out approach because one would have to add the
"--no-verify" flag or uncheck a box in the commit dialog to skip running
the checks.

thanks,
Jacek


wt., 27 cze 2023 o 01:55 Ekaterina Dimitrova <e....@gmail.com>
napisał(a):

> Thank you, Jacek, for starting the thread; those things are essential for
> developer productivity.
>
> I support the idea of opting out vs opting into checks. In my experience,
> it also makes things easier and faster during review time.
>
> If people have to opt-in - it is one more thing for new people to
> discover, and it will probably happen only during review time if they do
> not have access to Jenkins/paid CircleCI, etc.
>
> I also support consolidating all types of checks/analyses and running them
> together.
>
> Maxim’s suggestion about rat replacement sounds like a good improvement
> that can be explored (not part of what Jacek does here, though). Maxim, do
> you mind creating a ticket, please?
>
> Best regards,
> Ekaterina
>
> On Mon, 26 Jun 2023 at 17:04, Miklosovic, Stefan <
> Stefan.Miklosovic@netapp.com> wrote:
>
>> Yes, in this case, opting-out is better than opting-in. I feel like the
>> build process is quite versatile and one just picks what is necessary. I
>> never build docs, there is a flag for that. I turned off checkstyle because
>> I was fed up with that until Berenguer cached it and now I get ant jar with
>> checkstyle like under 10 seconds so I leave it on, which is great.
>>
>> Even though I feel like it is already flexible enough, grouping all
>> checkstyles and rats etc under one target seems like a good idea. From my
>> perspective, it is "all or nothing" so turning it all off until I am going
>> to push it so I want it all on is a good idea. I barely want to "just
>> checkstyle" in the middle of the development.
>>
>> I do not think that having a lot of flags is bad. I like that I have bash
>> aliases almost for everything and I bet folks have their tricks to get the
>> mundane stuff done.
>>
>> It would be pretty interesting to know the workflow of other people. I
>> think there would be a lot of insights how other people have it on a daily
>> basis when it comes to Cassandra development.
>>
>> ________________________________________
>> From: David Capwell <dc...@apple.com>
>> Sent: Monday, June 26, 2023 19:57
>> To: dev
>> Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations
>>
>> NetApp Security WARNING: This is an external email. Do not click links or
>> open attachments unless you recognize the sender and know the content is
>> safe.
>>
>>
>>
>> not running it automatically with the targets which devs usually run
>> locally.
>>
>> The checks tend to have an opt-out, such as -Dno-checkstyle=true… so its
>> really easy to setup your local environment to opt out what you do not care
>> about… I feel we should force people to opt-out rather than opt-in…
>>
>>
>>
>> On Jun 26, 2023, at 7:47 AM, Jacek Lewandowski <
>> lewandowski.jacek@gmail.com> wrote:
>>
>> That would work as well Brandon, basically what is proposed in
>> CASSANDRA-18618, that is "check" target, actually needs to build the
>> project to perform some verifications - I suppose running "ant check"
>> should be sufficient.
>>
>> - - -- --- ----- -------- -------------
>> Jacek Lewandowski
>>
>>
>> pon., 26 cze 2023 o 16:01 Brandon Williams <driftx@gmail.com<mailto:
>> driftx@gmail.com>> napisał(a):
>> The "artifacts" task is not quite the same since it builds other things
>> like docs, which significantly contributes to longer build time.  I don't
>> see why we couldn't add a new task that preserves the old behavior though,
>> "fulljar" or something like that.
>>
>> Kind Regards,
>> Brandon
>>
>>
>> On Mon, Jun 26, 2023 at 6:12 AM Jacek Lewandowski <
>> lewandowski.jacek@gmail.com<ma...@gmail.com>> wrote:
>> Yes, I've mentioned that there is a property we can set to skip
>> checkstyle.
>>
>> Currently such a goal is "artifacts" which basically validates everything.
>>
>>
>> - - -- --- ----- -------- -------------
>> Jacek Lewandowski
>>
>>
>> pon., 26 cze 2023 o 13:09 Mike Adamson <madamson@datastax.com<mailto:
>> madamson@datastax.com>> napisał(a):
>> While I like the idea of this because of added time these checks take, I
>> was under the impression that checkstyle (at least) can be disabled with a
>> flag.
>>
>> If we did do this, would it make sense to have a "release"  or "commit"
>> target (or some other name) that ran a full build with all checks that can
>> be used prior to pushing changes?
>>
>> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <berenguerblasi@gmail.com
>> <ma...@gmail.com>> wrote:
>>
>> I would prefer sthg that is totally transparent to me and not add one
>> more step I have to remember. Just to push/run CI to find out I missed it
>> and rinse and repeat... With the recent fix to checkstyle I am happy as
>> things stand atm. My 2cts
>>
>> On 26/6/23 8:43, Jacek Lewandowski wrote:
>> Hi,
>>
>> The context is that we currently have 3 checks in the build:
>> - Checkstyle,
>> - Eclipse-Warnings,
>> - RAT
>>
>> CheckStyle and RAT are executed with almost every target we run: build,
>> jar, test, test-some, testclasslist, etc.; on the other hand,
>> Eclipse-Warnings is executed automatically only with the artifacts target.
>>
>> Checkstyle currently uses some caching, so subsequent reruns without
>> cleaning the project validate only the modified files.
>>
>> Both CI - Jenkins and Circle forces running all checks.
>>
>> I want to discuss whether you are ok with extracting all checks to their
>> distinct target and not running it automatically with the targets which
>> devs usually run locally. In particular:
>>
>>
>>   *   "build", "jar", and all "test" targets would not trigger
>> CheckStyle, RAT or Eclipse-Warnings
>>   *   A new target "check" would trigger all CheckStyle, RAT, and
>> Eclipse-Warnings
>>   *   The new "check" target would be run along with the "artifacts"
>> target on Jenkins-CI, and it as a separate build step in CircleCI
>>
>> The rationale for that change is:
>>
>>   *   Running all the checks together would be more consistent, but
>> running all of them automatically with build and test targets could waste
>> time when we develop something locally, frequently rebuilding and running
>> tests.
>>   *   On the other hand, it would be more consistent if the build did
>> what we want - as a dev, when prototyping, I don't want to be forced to run
>> analysis (and potentially fix issues) whenever I want to build a project or
>> just run a single test.
>>   *   There are ways to avoid running checks automatically by specifying
>> some build properties. Though, the discussion is about the default behavior
>> - on the flip side, if one wants to run the checks along with the specified
>> target, they could add the "check" target to the command line.
>>
>> The rationale for keeping the checks running automatically with every
>> target is to reduce the likelihood of not running the checks locally before
>> pushing the branch and being surprised by failing CI soon after starting
>> the build.
>>
>> That could be fixed by running checks in a pre-push Git hook. There are
>> some benefits of this compared to the current behavior:
>>
>>   *   the checks would be run automatically only once
>>   *   they would be triggered even for those devs who do everything in
>> IDE and do not even touch Ant commands directly
>>
>> Checks can take time; to optimize that, they could be enforced locally to
>> verify only the modified files in the same way as we currently determine
>> the tests to be repeated for CircleCI.
>>
>> Thanks
>> - - -- --- ----- -------- -------------
>> Jacek Lewandowski
>>
>>
>> --
>> [DataStax Logo Square]<https://www.datastax.com/>     Mike Adamson
>> Engineering
>>
>> +1 650 389 6000<tel:16503896000> | datastax.com<https://www.datastax.com/
>> >
>> Find DataStax Online:   [LinkedIn Logo] <
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
>>   [Facebook Logo] <
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
>>   [Twitter Logo] <https://twitter.com/DataStax>    [RSS Feed] <
>> https://www.datastax.com/blog/rss.xml>    [Github Logo] <
>> https://github.com/datastax>
>>
>>
>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Ekaterina Dimitrova <e....@gmail.com>]

Thank you, Jacek, for starting the thread; those things are essential for
developer productivity.

I support the idea of opting out vs opting into checks. In my experience,
it also makes things easier and faster during review time.

If people have to opt-in - it is one more thing for new people to discover,
and it will probably happen only during review time if they do not have
access to Jenkins/paid CircleCI, etc.

I also support consolidating all types of checks/analyses and running them
together.

Maxim’s suggestion about rat replacement sounds like a good improvement
that can be explored (not part of what Jacek does here, though). Maxim, do
you mind creating a ticket, please?

Best regards,
Ekaterina

On Mon, 26 Jun 2023 at 17:04, Miklosovic, Stefan <
Stefan.Miklosovic@netapp.com> wrote:

> Yes, in this case, opting-out is better than opting-in. I feel like the
> build process is quite versatile and one just picks what is necessary. I
> never build docs, there is a flag for that. I turned off checkstyle because
> I was fed up with that until Berenguer cached it and now I get ant jar with
> checkstyle like under 10 seconds so I leave it on, which is great.
>
> Even though I feel like it is already flexible enough, grouping all
> checkstyles and rats etc under one target seems like a good idea. From my
> perspective, it is "all or nothing" so turning it all off until I am going
> to push it so I want it all on is a good idea. I barely want to "just
> checkstyle" in the middle of the development.
>
> I do not think that having a lot of flags is bad. I like that I have bash
> aliases almost for everything and I bet folks have their tricks to get the
> mundane stuff done.
>
> It would be pretty interesting to know the workflow of other people. I
> think there would be a lot of insights how other people have it on a daily
> basis when it comes to Cassandra development.
>
> ________________________________________
> From: David Capwell <dc...@apple.com>
> Sent: Monday, June 26, 2023 19:57
> To: dev
> Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations
>
> NetApp Security WARNING: This is an external email. Do not click links or
> open attachments unless you recognize the sender and know the content is
> safe.
>
>
>
> not running it automatically with the targets which devs usually run
> locally.
>
> The checks tend to have an opt-out, such as -Dno-checkstyle=true… so its
> really easy to setup your local environment to opt out what you do not care
> about… I feel we should force people to opt-out rather than opt-in…
>
>
>
> On Jun 26, 2023, at 7:47 AM, Jacek Lewandowski <
> lewandowski.jacek@gmail.com> wrote:
>
> That would work as well Brandon, basically what is proposed in
> CASSANDRA-18618, that is "check" target, actually needs to build the
> project to perform some verifications - I suppose running "ant check"
> should be sufficient.
>
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> pon., 26 cze 2023 o 16:01 Brandon Williams <driftx@gmail.com<mailto:
> driftx@gmail.com>> napisał(a):
> The "artifacts" task is not quite the same since it builds other things
> like docs, which significantly contributes to longer build time.  I don't
> see why we couldn't add a new task that preserves the old behavior though,
> "fulljar" or something like that.
>
> Kind Regards,
> Brandon
>
>
> On Mon, Jun 26, 2023 at 6:12 AM Jacek Lewandowski <
> lewandowski.jacek@gmail.com<ma...@gmail.com>> wrote:
> Yes, I've mentioned that there is a property we can set to skip checkstyle.
>
> Currently such a goal is "artifacts" which basically validates everything.
>
>
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> pon., 26 cze 2023 o 13:09 Mike Adamson <madamson@datastax.com<mailto:
> madamson@datastax.com>> napisał(a):
> While I like the idea of this because of added time these checks take, I
> was under the impression that checkstyle (at least) can be disabled with a
> flag.
>
> If we did do this, would it make sense to have a "release"  or "commit"
> target (or some other name) that ran a full build with all checks that can
> be used prior to pushing changes?
>
> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <berenguerblasi@gmail.com
> <ma...@gmail.com>> wrote:
>
> I would prefer sthg that is totally transparent to me and not add one more
> step I have to remember. Just to push/run CI to find out I missed it and
> rinse and repeat... With the recent fix to checkstyle I am happy as things
> stand atm. My 2cts
>
> On 26/6/23 8:43, Jacek Lewandowski wrote:
> Hi,
>
> The context is that we currently have 3 checks in the build:
> - Checkstyle,
> - Eclipse-Warnings,
> - RAT
>
> CheckStyle and RAT are executed with almost every target we run: build,
> jar, test, test-some, testclasslist, etc.; on the other hand,
> Eclipse-Warnings is executed automatically only with the artifacts target.
>
> Checkstyle currently uses some caching, so subsequent reruns without
> cleaning the project validate only the modified files.
>
> Both CI - Jenkins and Circle forces running all checks.
>
> I want to discuss whether you are ok with extracting all checks to their
> distinct target and not running it automatically with the targets which
> devs usually run locally. In particular:
>
>
>   *   "build", "jar", and all "test" targets would not trigger CheckStyle,
> RAT or Eclipse-Warnings
>   *   A new target "check" would trigger all CheckStyle, RAT, and
> Eclipse-Warnings
>   *   The new "check" target would be run along with the "artifacts"
> target on Jenkins-CI, and it as a separate build step in CircleCI
>
> The rationale for that change is:
>
>   *   Running all the checks together would be more consistent, but
> running all of them automatically with build and test targets could waste
> time when we develop something locally, frequently rebuilding and running
> tests.
>   *   On the other hand, it would be more consistent if the build did what
> we want - as a dev, when prototyping, I don't want to be forced to run
> analysis (and potentially fix issues) whenever I want to build a project or
> just run a single test.
>   *   There are ways to avoid running checks automatically by specifying
> some build properties. Though, the discussion is about the default behavior
> - on the flip side, if one wants to run the checks along with the specified
> target, they could add the "check" target to the command line.
>
> The rationale for keeping the checks running automatically with every
> target is to reduce the likelihood of not running the checks locally before
> pushing the branch and being surprised by failing CI soon after starting
> the build.
>
> That could be fixed by running checks in a pre-push Git hook. There are
> some benefits of this compared to the current behavior:
>
>   *   the checks would be run automatically only once
>   *   they would be triggered even for those devs who do everything in IDE
> and do not even touch Ant commands directly
>
> Checks can take time; to optimize that, they could be enforced locally to
> verify only the modified files in the same way as we currently determine
> the tests to be repeated for CircleCI.
>
> Thanks
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> --
> [DataStax Logo Square]<https://www.datastax.com/>     Mike Adamson
> Engineering
>
> +1 650 389 6000<tel:16503896000> | datastax.com<https://www.datastax.com/>
> Find DataStax Online:   [LinkedIn Logo] <
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
>   [Facebook Logo] <
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
>   [Twitter Logo] <https://twitter.com/DataStax>    [RSS Feed] <
> https://www.datastax.com/blog/rss.xml>    [Github Logo] <
> https://github.com/datastax>
>
>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

["Miklosovic, Stefan" <St...@netapp.com>]

Yes, in this case, opting-out is better than opting-in. I feel like the build process is quite versatile and one just picks what is necessary. I never build docs, there is a flag for that. I turned off checkstyle because I was fed up with that until Berenguer cached it and now I get ant jar with checkstyle like under 10 seconds so I leave it on, which is great.

Even though I feel like it is already flexible enough, grouping all checkstyles and rats etc under one target seems like a good idea. From my perspective, it is "all or nothing" so turning it all off until I am going to push it so I want it all on is a good idea. I barely want to "just checkstyle" in the middle of the development.

I do not think that having a lot of flags is bad. I like that I have bash aliases almost for everything and I bet folks have their tricks to get the mundane stuff done.

It would be pretty interesting to know the workflow of other people. I think there would be a lot of insights how other people have it on a daily basis when it comes to Cassandra development.

________________________________________
From: David Capwell <dc...@apple.com>
Sent: Monday, June 26, 2023 19:57
To: dev
Subject: Re: [DISCUSS] When to run CheckStyle and other verificiations

NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.



not running it automatically with the targets which devs usually run locally.

The checks tend to have an opt-out, such as -Dno-checkstyle=true… so its really easy to setup your local environment to opt out what you do not care about… I feel we should force people to opt-out rather than opt-in…



On Jun 26, 2023, at 7:47 AM, Jacek Lewandowski <le...@gmail.com> wrote:

That would work as well Brandon, basically what is proposed in CASSANDRA-18618, that is "check" target, actually needs to build the project to perform some verifications - I suppose running "ant check" should be sufficient.

- - -- --- ----- -------- -------------
Jacek Lewandowski


pon., 26 cze 2023 o 16:01 Brandon Williams <dr...@gmail.com>> napisał(a):
The "artifacts" task is not quite the same since it builds other things like docs, which significantly contributes to longer build time.  I don't see why we couldn't add a new task that preserves the old behavior though, "fulljar" or something like that.

Kind Regards,
Brandon


On Mon, Jun 26, 2023 at 6:12 AM Jacek Lewandowski <le...@gmail.com>> wrote:
Yes, I've mentioned that there is a property we can set to skip checkstyle.

Currently such a goal is "artifacts" which basically validates everything.


- - -- --- ----- -------- -------------
Jacek Lewandowski


pon., 26 cze 2023 o 13:09 Mike Adamson <ma...@datastax.com>> napisał(a):
While I like the idea of this because of added time these checks take, I was under the impression that checkstyle (at least) can be disabled with a flag.

If we did do this, would it make sense to have a "release"  or "commit" target (or some other name) that ran a full build with all checks that can be used prior to pushing changes?

On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <be...@gmail.com>> wrote:

I would prefer sthg that is totally transparent to me and not add one more step I have to remember. Just to push/run CI to find out I missed it and rinse and repeat... With the recent fix to checkstyle I am happy as things stand atm. My 2cts

On 26/6/23 8:43, Jacek Lewandowski wrote:
Hi,

The context is that we currently have 3 checks in the build:
- Checkstyle,
- Eclipse-Warnings,
- RAT

CheckStyle and RAT are executed with almost every target we run: build, jar, test, test-some, testclasslist, etc.; on the other hand, Eclipse-Warnings is executed automatically only with the artifacts target.

Checkstyle currently uses some caching, so subsequent reruns without cleaning the project validate only the modified files.

Both CI - Jenkins and Circle forces running all checks.

I want to discuss whether you are ok with extracting all checks to their distinct target and not running it automatically with the targets which devs usually run locally. In particular:


  *   "build", "jar", and all "test" targets would not trigger CheckStyle, RAT or Eclipse-Warnings
  *   A new target "check" would trigger all CheckStyle, RAT, and Eclipse-Warnings
  *   The new "check" target would be run along with the "artifacts" target on Jenkins-CI, and it as a separate build step in CircleCI

The rationale for that change is:

  *   Running all the checks together would be more consistent, but running all of them automatically with build and test targets could waste time when we develop something locally, frequently rebuilding and running tests.
  *   On the other hand, it would be more consistent if the build did what we want - as a dev, when prototyping, I don't want to be forced to run analysis (and potentially fix issues) whenever I want to build a project or just run a single test.
  *   There are ways to avoid running checks automatically by specifying some build properties. Though, the discussion is about the default behavior - on the flip side, if one wants to run the checks along with the specified target, they could add the "check" target to the command line.

The rationale for keeping the checks running automatically with every target is to reduce the likelihood of not running the checks locally before pushing the branch and being surprised by failing CI soon after starting the build.

That could be fixed by running checks in a pre-push Git hook. There are some benefits of this compared to the current behavior:

  *   the checks would be run automatically only once
  *   they would be triggered even for those devs who do everything in IDE and do not even touch Ant commands directly

Checks can take time; to optimize that, they could be enforced locally to verify only the modified files in the same way as we currently determine the tests to be repeated for CircleCI.

Thanks
- - -- --- ----- -------- -------------
Jacek Lewandowski


--
[DataStax Logo Square]<https://www.datastax.com/>     Mike Adamson
Engineering

+1 650 389 6000<tel:16503896000> | datastax.com<https://www.datastax.com/>
Find DataStax Online:   [LinkedIn Logo] <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>    [Facebook Logo] <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>    [Twitter Logo] <https://twitter.com/DataStax>    [RSS Feed] <https://www.datastax.com/blog/rss.xml>    [Github Logo] <https://github.com/datastax>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[David Capwell <dc...@apple.com>]

> not running it automatically with the targets which devs usually run locally.

The checks tend to have an opt-out, such as -Dno-checkstyle=true… so its really easy to setup your local environment to opt out what you do not care about… I feel we should force people to opt-out rather than opt-in… 



> On Jun 26, 2023, at 7:47 AM, Jacek Lewandowski <le...@gmail.com> wrote:
> 
> That would work as well Brandon, basically what is proposed in CASSANDRA-18618, that is "check" target, actually needs to build the project to perform some verifications - I suppose running "ant check" should be sufficient.  
> 
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
> 
> 
> pon., 26 cze 2023 o 16:01 Brandon Williams <driftx@gmail.com <ma...@gmail.com>> napisał(a):
>> The "artifacts" task is not quite the same since it builds other things like docs, which significantly contributes to longer build time.  I don't see why we couldn't add a new task that preserves the old behavior though, "fulljar" or something like that.
>> 
>> Kind Regards,
>> Brandon
>> 
>> 
>> On Mon, Jun 26, 2023 at 6:12 AM Jacek Lewandowski <lewandowski.jacek@gmail.com <ma...@gmail.com>> wrote:
>>> Yes, I've mentioned that there is a property we can set to skip checkstyle.
>>> 
>>> Currently such a goal is "artifacts" which basically validates everything.
>>> 
>>> 
>>> - - -- --- ----- -------- -------------
>>> Jacek Lewandowski
>>> 
>>> 
>>> pon., 26 cze 2023 o 13:09 Mike Adamson <madamson@datastax.com <ma...@datastax.com>> napisał(a):
>>>> While I like the idea of this because of added time these checks take, I was under the impression that checkstyle (at least) can be disabled with a flag. 
>>>> 
>>>> If we did do this, would it make sense to have a "release"  or "commit" target (or some other name) that ran a full build with all checks that can be used prior to pushing changes?
>>>> 
>>>> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <berenguerblasi@gmail.com <ma...@gmail.com>> wrote:
>>>>> I would prefer sthg that is totally transparent to me and not add one more step I have to remember. Just to push/run CI to find out I missed it and rinse and repeat... With the recent fix to checkstyle I am happy as things stand atm. My 2cts
>>>>> 
>>>>> On 26/6/23 8:43, Jacek Lewandowski wrote:
>>>>>> Hi,
>>>>>> 
>>>>>> The context is that we currently have 3 checks in the build:
>>>>>> - Checkstyle,
>>>>>> - Eclipse-Warnings,
>>>>>> - RAT
>>>>>> 
>>>>>> CheckStyle and RAT are executed with almost every target we run: build, jar, test, test-some, testclasslist, etc.; on the other hand, Eclipse-Warnings is executed automatically only with the artifacts target. 
>>>>>> 
>>>>>> Checkstyle currently uses some caching, so subsequent reruns without cleaning the project validate only the modified files.
>>>>>> 
>>>>>> Both CI - Jenkins and Circle forces running all checks.
>>>>>> 
>>>>>> I want to discuss whether you are ok with extracting all checks to their distinct target and not running it automatically with the targets which devs usually run locally. In particular:
>>>>>> 
>>>>>> "build", "jar", and all "test" targets would not trigger CheckStyle, RAT or Eclipse-Warnings
>>>>>> A new target "check" would trigger all CheckStyle, RAT, and Eclipse-Warnings
>>>>>> The new "check" target would be run along with the "artifacts" target on Jenkins-CI, and it as a separate build step in CircleCI
>>>>>> 
>>>>>> The rationale for that change is:
>>>>>> Running all the checks together would be more consistent, but running all of them automatically with build and test targets could waste time when we develop something locally, frequently rebuilding and running tests.
>>>>>> On the other hand, it would be more consistent if the build did what we want - as a dev, when prototyping, I don't want to be forced to run analysis (and potentially fix issues) whenever I want to build a project or just run a single test.
>>>>>> There are ways to avoid running checks automatically by specifying some build properties. Though, the discussion is about the default behavior - on the flip side, if one wants to run the checks along with the specified target, they could add the "check" target to the command line. 
>>>>>> 
>>>>>> The rationale for keeping the checks running automatically with every target is to reduce the likelihood of not running the checks locally before pushing the branch and being surprised by failing CI soon after starting the build. 
>>>>>> 
>>>>>> That could be fixed by running checks in a pre-push Git hook. There are some benefits of this compared to the current behavior:
>>>>>> the checks would be run automatically only once
>>>>>> they would be triggered even for those devs who do everything in IDE and do not even touch Ant commands directly
>>>>>> 
>>>>>> Checks can take time; to optimize that, they could be enforced locally to verify only the modified files in the same way as we currently determine the tests to be repeated for CircleCI.
>>>>>> 
>>>>>> Thanks
>>>>>> - - -- --- ----- -------- -------------
>>>>>> Jacek Lewandowski
>>>> 
>>>> 
>>>> -- 
>>>>  <https://www.datastax.com/>	Mike Adamson
>>>> Engineering
>>>> 
>>>> +1 650 389 6000 <tel:16503896000> | datastax.com <https://www.datastax.com/>
>>>> Find DataStax Online:	 <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>    <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>    <https://twitter.com/DataStax>    <https://www.datastax.com/blog/rss.xml>    <https://github.com/datastax>
>>>> 

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

That would work as well Brandon, basically what is proposed in
CASSANDRA-18618, that is "check" target, actually needs to build the
project to perform some verifications - I suppose running "ant check"
should be sufficient.

- - -- --- ----- -------- -------------
Jacek Lewandowski


pon., 26 cze 2023 o 16:01 Brandon Williams <dr...@gmail.com> napisał(a):

> The "artifacts" task is not quite the same since it builds other things
> like docs, which significantly contributes to longer build time.  I don't
> see why we couldn't add a new task that preserves the old behavior though,
> "fulljar" or something like that.
>
> Kind Regards,
> Brandon
>
>
> On Mon, Jun 26, 2023 at 6:12 AM Jacek Lewandowski <
> lewandowski.jacek@gmail.com> wrote:
>
>> Yes, I've mentioned that there is a property we can set to skip
>> checkstyle.
>>
>> Currently such a goal is "artifacts" which basically validates everything.
>>
>>
>> - - -- --- ----- -------- -------------
>> Jacek Lewandowski
>>
>>
>> pon., 26 cze 2023 o 13:09 Mike Adamson <ma...@datastax.com>
>> napisał(a):
>>
>>> While I like the idea of this because of added time these checks take, I
>>> was under the impression that checkstyle (at least) can be disabled with a
>>> flag.
>>>
>>> If we did do this, would it make sense to have a "release"  or "commit"
>>> target (or some other name) that ran a full build with all checks that can
>>> be used prior to pushing changes?
>>>
>>> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <be...@gmail.com>
>>> wrote:
>>>
>>>> I would prefer sthg that is totally transparent to me and not add one
>>>> more step I have to remember. Just to push/run CI to find out I missed it
>>>> and rinse and repeat... With the recent fix to checkstyle I am happy as
>>>> things stand atm. My 2cts
>>>> On 26/6/23 8:43, Jacek Lewandowski wrote:
>>>>
>>>> Hi,
>>>>
>>>>
>>>> The context is that we currently have 3 checks in the build:
>>>>
>>>> - Checkstyle,
>>>>
>>>> - Eclipse-Warnings,
>>>>
>>>> - RAT
>>>>
>>>>
>>>> CheckStyle and RAT are executed with almost every target we run: build,
>>>> jar, test, test-some, testclasslist, etc.; on the other hand,
>>>> Eclipse-Warnings is executed automatically only with the artifacts target.
>>>>
>>>>
>>>> Checkstyle currently uses some caching, so subsequent reruns without
>>>> cleaning the project validate only the modified files.
>>>>
>>>>
>>>> Both CI - Jenkins and Circle forces running all checks.
>>>>
>>>>
>>>> I want to discuss whether you are ok with extracting all checks to
>>>> their distinct target and not running it automatically with the targets
>>>> which devs usually run locally. In particular:
>>>>
>>>>
>>>>
>>>>    - "build", "jar", and all "test" targets would not trigger
>>>>    CheckStyle, RAT or Eclipse-Warnings
>>>>    - A new target "check" would trigger all CheckStyle, RAT, and
>>>>    Eclipse-Warnings
>>>>    - The new "check" target would be run along with the "artifacts"
>>>>    target on Jenkins-CI, and it as a separate build step in CircleCI
>>>>
>>>>
>>>> The rationale for that change is:
>>>>
>>>>    - Running all the checks together would be more consistent, but
>>>>    running all of them automatically with build and test targets could waste
>>>>    time when we develop something locally, frequently rebuilding and running
>>>>    tests.
>>>>    - On the other hand, it would be more consistent if the build did
>>>>    what we want - as a dev, when prototyping, I don't want to be forced to run
>>>>    analysis (and potentially fix issues) whenever I want to build a project or
>>>>    just run a single test.
>>>>    - There are ways to avoid running checks automatically by
>>>>    specifying some build properties. Though, the discussion is about the
>>>>    default behavior - on the flip side, if one wants to run the checks along
>>>>    with the specified target, they could add the "check" target to the command
>>>>    line.
>>>>
>>>>
>>>> The rationale for keeping the checks running automatically with every
>>>> target is to reduce the likelihood of not running the checks locally before
>>>> pushing the branch and being surprised by failing CI soon after starting
>>>> the build.
>>>>
>>>>
>>>> That could be fixed by running checks in a pre-push Git hook. There are
>>>> some benefits of this compared to the current behavior:
>>>>
>>>>    - the checks would be run automatically only once
>>>>    - they would be triggered even for those devs who do everything in
>>>>    IDE and do not even touch Ant commands directly
>>>>
>>>>
>>>> Checks can take time; to optimize that, they could be enforced locally
>>>> to verify only the modified files in the same way as we currently determine
>>>> the tests to be repeated for CircleCI.
>>>>
>>>> Thanks
>>>> - - -- --- ----- -------- -------------
>>>> Jacek Lewandowski
>>>>
>>>>
>>>
>>> --
>>> [image: DataStax Logo Square] <https://www.datastax.com/> *Mike Adamson*
>>> Engineering
>>>
>>> +1 650 389 6000 <16503896000> | datastax.com <https://www.datastax.com/>
>>> Find DataStax Online: [image: LinkedIn Logo]
>>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
>>>    [image: Facebook Logo]
>>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
>>>    [image: Twitter Logo] <https://twitter.com/DataStax>   [image: RSS
>>> Feed] <https://www.datastax.com/blog/rss.xml>   [image: Github Logo]
>>> <https://github.com/datastax>
>>>
>>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Brandon Williams <dr...@gmail.com>]

The "artifacts" task is not quite the same since it builds other things
like docs, which significantly contributes to longer build time.  I don't
see why we couldn't add a new task that preserves the old behavior though,
"fulljar" or something like that.

Kind Regards,
Brandon


On Mon, Jun 26, 2023 at 6:12 AM Jacek Lewandowski <
lewandowski.jacek@gmail.com> wrote:

> Yes, I've mentioned that there is a property we can set to skip checkstyle.
>
> Currently such a goal is "artifacts" which basically validates everything.
>
>
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>
> pon., 26 cze 2023 o 13:09 Mike Adamson <ma...@datastax.com> napisał(a):
>
>> While I like the idea of this because of added time these checks take, I
>> was under the impression that checkstyle (at least) can be disabled with a
>> flag.
>>
>> If we did do this, would it make sense to have a "release"  or "commit"
>> target (or some other name) that ran a full build with all checks that can
>> be used prior to pushing changes?
>>
>> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <be...@gmail.com>
>> wrote:
>>
>>> I would prefer sthg that is totally transparent to me and not add one
>>> more step I have to remember. Just to push/run CI to find out I missed it
>>> and rinse and repeat... With the recent fix to checkstyle I am happy as
>>> things stand atm. My 2cts
>>> On 26/6/23 8:43, Jacek Lewandowski wrote:
>>>
>>> Hi,
>>>
>>>
>>> The context is that we currently have 3 checks in the build:
>>>
>>> - Checkstyle,
>>>
>>> - Eclipse-Warnings,
>>>
>>> - RAT
>>>
>>>
>>> CheckStyle and RAT are executed with almost every target we run: build,
>>> jar, test, test-some, testclasslist, etc.; on the other hand,
>>> Eclipse-Warnings is executed automatically only with the artifacts target.
>>>
>>>
>>> Checkstyle currently uses some caching, so subsequent reruns without
>>> cleaning the project validate only the modified files.
>>>
>>>
>>> Both CI - Jenkins and Circle forces running all checks.
>>>
>>>
>>> I want to discuss whether you are ok with extracting all checks to their
>>> distinct target and not running it automatically with the targets which
>>> devs usually run locally. In particular:
>>>
>>>
>>>
>>>    - "build", "jar", and all "test" targets would not trigger
>>>    CheckStyle, RAT or Eclipse-Warnings
>>>    - A new target "check" would trigger all CheckStyle, RAT, and
>>>    Eclipse-Warnings
>>>    - The new "check" target would be run along with the "artifacts"
>>>    target on Jenkins-CI, and it as a separate build step in CircleCI
>>>
>>>
>>> The rationale for that change is:
>>>
>>>    - Running all the checks together would be more consistent, but
>>>    running all of them automatically with build and test targets could waste
>>>    time when we develop something locally, frequently rebuilding and running
>>>    tests.
>>>    - On the other hand, it would be more consistent if the build did
>>>    what we want - as a dev, when prototyping, I don't want to be forced to run
>>>    analysis (and potentially fix issues) whenever I want to build a project or
>>>    just run a single test.
>>>    - There are ways to avoid running checks automatically by specifying
>>>    some build properties. Though, the discussion is about the default behavior
>>>    - on the flip side, if one wants to run the checks along with the specified
>>>    target, they could add the "check" target to the command line.
>>>
>>>
>>> The rationale for keeping the checks running automatically with every
>>> target is to reduce the likelihood of not running the checks locally before
>>> pushing the branch and being surprised by failing CI soon after starting
>>> the build.
>>>
>>>
>>> That could be fixed by running checks in a pre-push Git hook. There are
>>> some benefits of this compared to the current behavior:
>>>
>>>    - the checks would be run automatically only once
>>>    - they would be triggered even for those devs who do everything in
>>>    IDE and do not even touch Ant commands directly
>>>
>>>
>>> Checks can take time; to optimize that, they could be enforced locally
>>> to verify only the modified files in the same way as we currently determine
>>> the tests to be repeated for CircleCI.
>>>
>>> Thanks
>>> - - -- --- ----- -------- -------------
>>> Jacek Lewandowski
>>>
>>>
>>
>> --
>> [image: DataStax Logo Square] <https://www.datastax.com/> *Mike Adamson*
>> Engineering
>>
>> +1 650 389 6000 <16503896000> | datastax.com <https://www.datastax.com/>
>> Find DataStax Online: [image: LinkedIn Logo]
>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
>>    [image: Facebook Logo]
>> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
>>    [image: Twitter Logo] <https://twitter.com/DataStax>   [image: RSS
>> Feed] <https://www.datastax.com/blog/rss.xml>   [image: Github Logo]
>> <https://github.com/datastax>
>>
>>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Jacek Lewandowski <le...@gmail.com>]

Yes, I've mentioned that there is a property we can set to skip checkstyle.

Currently such a goal is "artifacts" which basically validates everything.


- - -- --- ----- -------- -------------
Jacek Lewandowski


pon., 26 cze 2023 o 13:09 Mike Adamson <ma...@datastax.com> napisał(a):

> While I like the idea of this because of added time these checks take, I
> was under the impression that checkstyle (at least) can be disabled with a
> flag.
>
> If we did do this, would it make sense to have a "release"  or "commit"
> target (or some other name) that ran a full build with all checks that can
> be used prior to pushing changes?
>
> On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <be...@gmail.com>
> wrote:
>
>> I would prefer sthg that is totally transparent to me and not add one
>> more step I have to remember. Just to push/run CI to find out I missed it
>> and rinse and repeat... With the recent fix to checkstyle I am happy as
>> things stand atm. My 2cts
>> On 26/6/23 8:43, Jacek Lewandowski wrote:
>>
>> Hi,
>>
>>
>> The context is that we currently have 3 checks in the build:
>>
>> - Checkstyle,
>>
>> - Eclipse-Warnings,
>>
>> - RAT
>>
>>
>> CheckStyle and RAT are executed with almost every target we run: build,
>> jar, test, test-some, testclasslist, etc.; on the other hand,
>> Eclipse-Warnings is executed automatically only with the artifacts target.
>>
>>
>> Checkstyle currently uses some caching, so subsequent reruns without
>> cleaning the project validate only the modified files.
>>
>>
>> Both CI - Jenkins and Circle forces running all checks.
>>
>>
>> I want to discuss whether you are ok with extracting all checks to their
>> distinct target and not running it automatically with the targets which
>> devs usually run locally. In particular:
>>
>>
>>
>>    - "build", "jar", and all "test" targets would not trigger
>>    CheckStyle, RAT or Eclipse-Warnings
>>    - A new target "check" would trigger all CheckStyle, RAT, and
>>    Eclipse-Warnings
>>    - The new "check" target would be run along with the "artifacts"
>>    target on Jenkins-CI, and it as a separate build step in CircleCI
>>
>>
>> The rationale for that change is:
>>
>>    - Running all the checks together would be more consistent, but
>>    running all of them automatically with build and test targets could waste
>>    time when we develop something locally, frequently rebuilding and running
>>    tests.
>>    - On the other hand, it would be more consistent if the build did
>>    what we want - as a dev, when prototyping, I don't want to be forced to run
>>    analysis (and potentially fix issues) whenever I want to build a project or
>>    just run a single test.
>>    - There are ways to avoid running checks automatically by specifying
>>    some build properties. Though, the discussion is about the default behavior
>>    - on the flip side, if one wants to run the checks along with the specified
>>    target, they could add the "check" target to the command line.
>>
>>
>> The rationale for keeping the checks running automatically with every
>> target is to reduce the likelihood of not running the checks locally before
>> pushing the branch and being surprised by failing CI soon after starting
>> the build.
>>
>>
>> That could be fixed by running checks in a pre-push Git hook. There are
>> some benefits of this compared to the current behavior:
>>
>>    - the checks would be run automatically only once
>>    - they would be triggered even for those devs who do everything in
>>    IDE and do not even touch Ant commands directly
>>
>>
>> Checks can take time; to optimize that, they could be enforced locally to
>> verify only the modified files in the same way as we currently determine
>> the tests to be repeated for CircleCI.
>>
>> Thanks
>> - - -- --- ----- -------- -------------
>> Jacek Lewandowski
>>
>>
>
> --
> [image: DataStax Logo Square] <https://www.datastax.com/> *Mike Adamson*
> Engineering
>
> +1 650 389 6000 <16503896000> | datastax.com <https://www.datastax.com/>
> Find DataStax Online: [image: LinkedIn Logo]
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
>    [image: Facebook Logo]
> <https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
>    [image: Twitter Logo] <https://twitter.com/DataStax>   [image: RSS
> Feed] <https://www.datastax.com/blog/rss.xml>   [image: Github Logo]
> <https://github.com/datastax>
>
>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Mike Adamson <ma...@datastax.com>]

While I like the idea of this because of added time these checks take, I
was under the impression that checkstyle (at least) can be disabled with a
flag.

If we did do this, would it make sense to have a "release"  or "commit"
target (or some other name) that ran a full build with all checks that can
be used prior to pushing changes?

On Mon, 26 Jun 2023 at 08:35, Berenguer Blasi <be...@gmail.com>
wrote:

> I would prefer sthg that is totally transparent to me and not add one more
> step I have to remember. Just to push/run CI to find out I missed it and
> rinse and repeat... With the recent fix to checkstyle I am happy as things
> stand atm. My 2cts
> On 26/6/23 8:43, Jacek Lewandowski wrote:
>
> Hi,
>
>
> The context is that we currently have 3 checks in the build:
>
> - Checkstyle,
>
> - Eclipse-Warnings,
>
> - RAT
>
>
> CheckStyle and RAT are executed with almost every target we run: build,
> jar, test, test-some, testclasslist, etc.; on the other hand,
> Eclipse-Warnings is executed automatically only with the artifacts target.
>
>
> Checkstyle currently uses some caching, so subsequent reruns without
> cleaning the project validate only the modified files.
>
>
> Both CI - Jenkins and Circle forces running all checks.
>
>
> I want to discuss whether you are ok with extracting all checks to their
> distinct target and not running it automatically with the targets which
> devs usually run locally. In particular:
>
>
>
>    - "build", "jar", and all "test" targets would not trigger CheckStyle,
>    RAT or Eclipse-Warnings
>    - A new target "check" would trigger all CheckStyle, RAT, and
>    Eclipse-Warnings
>    - The new "check" target would be run along with the "artifacts"
>    target on Jenkins-CI, and it as a separate build step in CircleCI
>
>
> The rationale for that change is:
>
>    - Running all the checks together would be more consistent, but
>    running all of them automatically with build and test targets could waste
>    time when we develop something locally, frequently rebuilding and running
>    tests.
>    - On the other hand, it would be more consistent if the build did what
>    we want - as a dev, when prototyping, I don't want to be forced to run
>    analysis (and potentially fix issues) whenever I want to build a project or
>    just run a single test.
>    - There are ways to avoid running checks automatically by specifying
>    some build properties. Though, the discussion is about the default behavior
>    - on the flip side, if one wants to run the checks along with the specified
>    target, they could add the "check" target to the command line.
>
>
> The rationale for keeping the checks running automatically with every
> target is to reduce the likelihood of not running the checks locally before
> pushing the branch and being surprised by failing CI soon after starting
> the build.
>
>
> That could be fixed by running checks in a pre-push Git hook. There are
> some benefits of this compared to the current behavior:
>
>    - the checks would be run automatically only once
>    - they would be triggered even for those devs who do everything in IDE
>    and do not even touch Ant commands directly
>
>
> Checks can take time; to optimize that, they could be enforced locally to
> verify only the modified files in the same way as we currently determine
> the tests to be repeated for CircleCI.
>
> Thanks
> - - -- --- ----- -------- -------------
> Jacek Lewandowski
>
>

-- 
[image: DataStax Logo Square] <https://www.datastax.com/> *Mike Adamson*
Engineering

+1 650 389 6000 <16503896000> | datastax.com <https://www.datastax.com/>
Find DataStax Online: [image: LinkedIn Logo]
<https://urldefense.proofpoint.com/v2/url?u=https-3A__www.linkedin.com_company_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=akx0E6l2bnTjOvA-YxtonbW0M4b6bNg4nRwmcHNDo4Q&e=>
   [image: Facebook Logo]
<https://urldefense.proofpoint.com/v2/url?u=https-3A__www.facebook.com_datastax&d=DwMFaQ&c=adz96Xi0w1RHqtPMowiL2g&r=IFj3MdIKYLLXIUhYdUGB0cTzTlxyCb7_VUmICBaYilU&m=uHzE4WhPViSF0rsjSxKhfwGDU1Bo7USObSc_aIcgelo&s=ncMlB41-6hHuqx-EhnM83-KVtjMegQ9c2l2zDzHAxiU&e=>
   [image: Twitter Logo] <https://twitter.com/DataStax>   [image: RSS Feed]
<https://www.datastax.com/blog/rss.xml>   [image: Github Logo]
<https://github.com/datastax>

Re: [DISCUSS] When to run CheckStyle and other verificiations

[Berenguer Blasi <be...@gmail.com>]

I would prefer sthg that is totally transparent to me and not add one 
more step I have to remember. Just to push/run CI to find out I missed 
it and rinse and repeat... With the recent fix to checkstyle I am happy 
as things stand atm. My 2cts

On 26/6/23 8:43, Jacek Lewandowski wrote:
>
> Hi,
>
>
> The context is that we currently have 3 checks in the build:
>
> - Checkstyle,
>
> - Eclipse-Warnings,
>
> - RAT
>
>
> CheckStyle and RAT are executed with almost every target we run: 
> build, jar, test, test-some, testclasslist, etc.; on the other hand, 
> Eclipse-Warnings is executed automatically only with the artifacts 
> target.
>
>
> Checkstyle currently uses some caching, so subsequent reruns without 
> cleaning the project validate only the modified files.
>
>
> Both CI - Jenkins and Circle forces running all checks.
>
>
> I want to discuss whether you are ok with extracting all checks to 
> their distinct target and not running it automatically with the 
> targets which devs usually run locally. In particular:
>
>
>   * "build", "jar", and all "test" targets would not trigger
>     CheckStyle, RAT or Eclipse-Warnings
>   * A new target "check" would trigger all CheckStyle, RAT, and
>     Eclipse-Warnings
>   * The new "check" target would be run along with the "artifacts"
>     target on Jenkins-CI, and it as a separate build step in CircleCI
>
>
> The rationale for that change is:
>
>   * Running all the checks together would be more consistent, but
>     running all of them automatically with build and test targets
>     could waste time when we develop something locally, frequently
>     rebuilding and running tests.
>   * On the other hand, it would be more consistent if the build did
>     what we want - as a dev, when prototyping, I don't want to be
>     forced to run analysis (and potentially fix issues) whenever I
>     want to build a project or just run a single test.
>   * There are ways to avoid running checks automatically by specifying
>     some build properties. Though, the discussion is about the default
>     behavior - on the flip side, if one wants to run the checks along
>     with the specified target, they could add the "check" target to
>     the command line.
>
>
> The rationale for keeping the checks running automatically with every 
> target is to reduce the likelihood of not running the checks locally 
> before pushing the branch and being surprised by failing CI soon after 
> starting the build.
>
>
> That could be fixed by running checks in a pre-push Git hook. There 
> are some benefits of this compared to the current behavior:
>
>   * the checks would be run automatically only once
>   * they would be triggered even for those devs who do everything in
>     IDE and do not even touch Ant commands directly
>
>
> Checks can take time; to optimize that, they could be enforced locally 
> to verify only the modified files in the same way as we currently 
> determine the tests to be repeated for CircleCI.
>
>
> Thanks
> - - -- --- ----- -------- -------------
> Jacek Lewandowski