Clang format

[miguelteixeiraa]

ref: #170

I was thinking about the following options:

• Making run-clang-format.sh a "cmake script" (to be used like cmake -P clang-format.cmake)
• Having a Makefile in the root of the repository to add scripts like this
• Leave it as it is right there in the tools folder

Any thoughts?

[anonrig]

I'd go with something like execute_process that runs on every build step. Depending on the environment variable of github workflow, you can throw an error on CI builds to say that "linting is required".

If you want to run directly the format command using cmake, make as add_custom_command() function.

[anonrig]

Hi @miguelteixeiraa, would you like to complete this? We can open another pull request to fix all the linter related issues.

[miguelteixeiraa]

I could complete that, but if you already have everything ready, no problem! You can push what you have and we can close this one! : )

[anonrig]

I could complete that, but if you already have everything ready, no problem! You can push what you have and we can close this one! : )

I haven't worked on it, I'm just interested :-)

[miguelteixeiraa]

I made the execution of the "check" function/script be conditioned to a variable called IS_CI, and the default "format" script/function for every time it is not the CI.. if I turn it to be a custom command, it would be better (talking about the "format")?

[anonrig]

I'm happy with this pull request! Just updating or adding a GitHub workflow would be sufficient to merge

[anonrig]

Can you also update one of the github workflows to add a IS_CI environment variable?

[anonrig]

Can you remove the fixes to all files? It will create huge conflict with the aggregator pull request. Lets merge it with a failing test

[miguelteixeiraa]

Can you remove the fixes to all files? It will create huge conflict with the aggregator pull request. Lets merge it with a failing test

Sure thing! I'm just trying to figure out why it still falling even formatting all files. I'll rollback after that.

[miguelteixeiraa]

It doesn't seem to be a big problem.. There is a major version difference between the clang-format version installed by default in ubuntu-latest and the one I have running locally (15.0.6 on my Mac, and 14.x.x on Ubuntu-latest probably). So this is formatting differently in the two places.
Ie we need to choose a version of clang-format to use.

[miguelteixeiraa]

Thinking of downloading a specific version of clang-format and keeping it in a folder (which will be in .gitignore) callend ".ada_extras" (please let me know if you have naming suggestions in case we follow this path).. and then make the script to use the downloaded clang-format binary instead of the one that is in the PATH....

Any other suggestions? Thoughts?

[miguelteixeiraa]

• I fixed a clang-format version (15) and the github actions workflow is already modified to consider this version.
• Formatting will only happen automatically on each build if the FORMAT_ENABLED environment variable is set.
• The check (without formatting) will only happen if the environment variable LINT_AND_FORMAT_CHECK is set (it is set/fixed in the CI/lint-and-format workflow).
• If the script runs (both for checking or for formatting) and the executable clang-format installed has a different version than the script's default (or the one passed as an argument for the script), the script should produce an error.

That's how everything is working now : )

[miguelteixeiraa]

Now I'll go back to work on one of the tests PR because they are probably celebrating their birthday at this point 😅

[miguelteixeiraa]

@lemire @anonrig Do you need me to do something else on this one before merging?

[anonrig]

@lemire What do you think?

[lemire]

I leave it to you.

[anonrig]

@miguelteixeiraa Can we update idna with these changes too?

[miguelteixeiraa]

@miguelteixeiraa Can we update idna with these changes too?

Sure thing!