k8ssandra / management-api-for-apache-cassandra

RESTful / Secure Management Sidecar for Apache Cassandra
Apache License 2.0
72 stars 51 forks source link

mvn format command doesn't format all files #387

Open Miles-Garnsey opened 1 year ago

Miles-Garnsey commented 1 year ago

When we run mvn com.coveo:fmt-maven-plugin:format as recommended by a failing build output, it does not format all files and CI will continue to fail.

Instead, one must also run mvn xml-format:xml-format -P dse,dse7,trunk in order to get CI to run.

This is confusing for new developers and (even for those of us who know about it) is an easy step to miss. A single command should run both steps. Also, both steps should be run in a pre-commit hook by git, so that developers aren't forced to frequently amend their commits before re-pushing.

┆Issue is synchronized with this Jira Story by Unito

emerkle826 commented 1 year ago

The dse, dse7, and trunk Maven profiles aren't enabled by default so you have to enable them on the command line (or with IDE tooling) if you want maven targets to run on them. The DSE profiles in particular require access to DSE artifacts, and not everyone will have the required access. That's why those profiles aren't enabled by default. The trunk profile is for Cassandra trunk builds, which by nature may be unstable.

As for formatting, it is documented in the [README}(https://github.com/k8ssandra/management-api-for-apache-cassandra#checking-the-format). You can simply run

mvn fmt:check -P dse,dse7,trunk

Before committing your changes. Also, the format is checked during the compile phase. If any files are not in compliance, the compile will fail and tell you which files are not in compliance.

I can add a pre-commit hook if that makes things better.

Miles-Garnsey commented 1 year ago

I guess the reason we're having this problem in this repo specifically is that some contributors aren't on Linux and therefore aren't running tests/compilation locally, so the formatting checks can get missed. Erik and I have agreed that in this specific case a pre-commit hook which does the formatting locally before anything is ever committed is probably going to speed things up a little.