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 <stefan.mikloso...@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 <lewandowski.ja...@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.dimitr...@gmail.com > <mailto: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<mailto: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<mailto: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<mailto: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><mailto: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><mailto: > 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><mailto: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><mailto: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/<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>> > > >
