On Mon, Oct 30 08:35:36 UTC 2017, Miklos Vajna wrote: >On Fri, Oct 27, 2017 at 04:49:55PM +0200, Jan Holesovsky wrote: >> Good point - another reason why not to do the rewriting completely >> automatically server side I guess. >> >> But still, I see the Thorsten's point why it would be easier for people >> in many cases; that's why I proposed the 'automatic, but ending up as >> an additional changeset' way, that at least gives a chance to inspect & >> do something about that. > >I see the benefit of that, and I can accept that as a compromise, >though >I fear a bit it introduces another set of problems: [...]
Many OSS projects in the wild enforce code styles. Some projects, even enforce multiple different code style checks for different languages and even different checks for a single language. One example is tensoflow: https://github.com/tensorflow/tensorflow/blob/c44f67a7ed5870fe8a1c0d625 7ce597ca2ef7564/tensorflow/tools/ci_build/ci_sanity.sh is using: * pylint * pep8 * buildifier (Bazel BUILD files) * clang_format_check The way it's implemented, obviously, is the CI build check. In Gerrit Code Review, we've added dedicates Gerrit Label: Code-Style, that blocks the change, when voted -1, and +1 is required for a change to be submittable. I would not recommend any magic, neither on the client nor on the server (Gerrit) side: no pre-commit hooks, no automagical new patch set creation, in case Code-Style check(s) are failed (say both checks verify and code-style are failed, and creation of new patch set would not fix the root cause for verification failure, like compiler error or similar; the patch set was created by human and should be fxied by human). Of course, a script should be provided to run the check at any time by contributor, it should not be integrated in the commit hook. That how I would expect it to be implemented. When I push to gerrit, see below, all checks should be run as part of Gerrit Code-Syle verification job. For those who don't use gerrit, and push directly to master, well, they should check the style on their own, but hey, they already verify on their own the commit on 4 platform locally, so running locally the code-style checks shouldn't be a problem. Next reason why client side pre-commit hook wouldn't solve all use cases: it's possible to create a modification in Gerrit, using inline edit feature. Any client pre-commit hooks wouldn't be executed in that use case. My advice would be to add Code-Style label to gerrit, say Code-Style, like we did in Gerrit Code Review upstream, in this change: [1], with the range [-1,+1]. In additional add new Jenkins job that is running the checks and is voting on the new label. So that a normal change in Gerrit Code Review project must now have max positive votes on all these labels, to be submittable, e.g.: [2]. Code-Review +2 Code-Style +1 Library-Compliance +1 Verified +1 And no, I'm not volunteering to do the job. * [1] https://gerrit-review.googlesource.com/#/c/gerrit/+/108216 * [2] https://gerrit-review.googlesource.com/#/c/gerrit/+/122170 _______________________________________________ LibreOffice mailing list LibreOffice@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice
