> 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 <m...@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).
