Add fmt-check to make checks-backend for CI

[yardenshoham]

• Following Run make fmt #21437 I realized the build system isn't actually checking that the code is formatted

[silverwind]

Instead of adding another drone step, just add it to checks-backend as a dependency:

gitea/Makefile

Line 337 in 9dc264a

checks-backend: tidy-check swagger-check swagger-validate

E.g.

 checks-backend: tidy-check swagger-check fmt-check swagger-validate

[silverwind]

I think we also need to add fmt to the dependencies of fmt-check, otherwise it will never see a diff.

[yardenshoham]

Hmmm, I see, perhaps having a different way to handle templates formatting is not the best way to go. Simply having fmt being called during fmt-check drastically changes its behavior. Wrong Go formatting won't be detected in Drone at all. The best thing to do would be to have gitea-fmt include template formatting.

[silverwind]

I'm not sure what you're getting at, but we use the same strategy already for other things like tidy-check which has the tidy dependency. When running tidy-check, make will just trigger the tidy target before it, nur "during" it.

So I think having fmt-check: fmt on line 315 is totally fine.

[yardenshoham]

Suppose I commit a changeset with wrongly formatted Go code and correctly formatted templates, fmt runs, formatting my Go code on the CI filesystem and then fmt-check runs (reporting no errors as fmt formatted my bad Go code and templates is formatted correctly) and exits with success.

[silverwind]

Ah, I see fmt-check also runs the go formatting again, I missed that. So the solution should then be remove the go formatting in fmt-check, so that only the diffing is left, then add the dependency which in turn will make it work just like tidy-check does while keeping DRY as well :)

[silverwind]

I suppose you can just reduce to a single git diff call as well:

@diff=$$(git diff $(GO_SOURCES) templates); \
if [ -n "$$diff" ]; then \
  echo "Please run 'make fmt' and commit the result:"; \
  echo "$${diff}"; \
  exit 1; \
fi

[yardenshoham]

How about we merge this as-is and open an issue on how to handle the actual fmt-check logic? This PR is only about adding fmt-check to CI

[silverwind]

#21458 for the proposed enhancements. Fine with me to land this before.

[silverwind]

I suggest we just land #21458 instead of this. The order of targets is different here, so if this lands my PR would conflict.