only fail qmlformat stage if Qt >= 5.15 is used

[daschuer]

this uses "moc -v" for version check, because this is needed for building anyway.

[Holzhaus]

Not sure if this is a good idea. It's more convenient for people that merge main into their PR, but may lead to skin change PRs with broken qml formatting without the contributor expecting it. And if we ever accidently have a too old Qt version on CI, we won't notice qml formatting issues.

[daschuer]

Not sure if this is a good idea.

It just does automatically what is suggested by the error message. The contributor has no chance to fix it on such a system.
Verifying the qt version on our CI is another topic.

[Holzhaus]

Not sure if this is a good idea.

It just does automatically what is suggested by the error message. The contributor has no chance to fix it on such a system.
Verifying the qt version on our CI is another topic.

Before:

1. User installs pre-commit hooks
2. User makes QML changes
3. User commits, sees the error message and either updates Qt or skips the hook (in that case CI failure is expected)

After:

1. User installs pre-commit hooks
2. User makes QML changes
3. User commits. The qmlformat hook passes.
4. User opens PR and is confused that the pre-commit hook suddenly fails although everything "worked" locally -> "pre-commit is broken!!!"

[uklotzde]

I agree with Jan and would prefer the safe (and future proof) version.

[daschuer]

I think my version is the future proved one, please verify:

Before:
User installs pre-commit hooks
User makes QML changes
User commits, sees the error message and permanently disabled the hook.
He is facing unrelated and unnecessary hassle, nothing to do with the one liner fix he is trying to commit ...
(User will unlikely intall a home build Qt update just because of qmlformat)
Later the distro will updating QT
User will keep the hook disabled

After:
User installs pre-commit hooks
User makes QML changes
User commits. The qmlformat hook passes.
User opens PR and is confused that the pre-commit hook suddenly fails ...
(This is why we have CI, it may fail for plenty of reasons on targets that differ from the dev machine.)
Once the dev machine is automatically updated by the distro to QT 5.14 it works automatically without additional actions.

[daschuer]

Uh ... this is really helpful. Imagine you are on qt 5.12 ...
Did you reconsider this?

[uklotzde]

@Holzhaus Merge?

@daschuer Maybe we should add a TODO that allows us to better identify and remove obsolete code once we have switched to Qt >= 5.15? All this new code is only there for the purpose of backwards compatibility. The expression (5, 15) is not easily identifiable as the Qt version when searching for it.

[Holzhaus]

Uh ... this is really helpful. Imagine you are on qt 5.12 ...
Did you reconsider this?

As I said, the proper fix would be to run echo "export SKIP=qmlformat" >> ~/.profile. We could even add this to the error message. But I won't stand in the way if you really want to merge this.

[daschuer]

Yes, I really like to merge it, because of the good first time experience and the feature that it is automatically enabled once QT 5.14 is installed.

[uklotzde]

I am not excited that we need to add another ugly workaround for Ubuntu. At least its just a build script.

LGTM