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.dimitr...@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.mikloso...@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 <dcapw...@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.ja...@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 <dri...@gmail.com<mailto: >> dri...@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.ja...@gmail.com<mailto:lewandowski.ja...@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 <madam...@datastax.com<mailto: >> madam...@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 <berenguerbl...@gmail.com >> <mailto:berenguerbl...@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> >> >> >>
