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 <lewandowski.ja...@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 <dcapw...@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 <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). >> >> >>
