ci: add formatting check with each PR

[antonbaliasnikov]

Code Review Checklist

Purpose

Adds diff formatting check with each PR.

Ticket Number

Fixes CPR-1054

Requirements

• Have the requirements been met?
• Have stakeholder(s) approved the change?

Implementation

• Does this code change accomplish what it is supposed to do?
• Can this solution be simplified?
• Does this change add unwanted compile-time or run-time dependencies?
• Could an additional framework, API, library, or service improve the solution?
• Could we reuse part of LLVM instead of implementing the patch or a part of it?
• Is the code at the right abstraction level?
• Is the code modular enough?
• Can a better solution be found in terms of maintainability, readability, performance, or security?
• Does similar functionality already exist in the codebase? If yes, why isn’t it reused?
• Are there any best practices, design patterns or language-specific patterns that could substantially improve this code?

Logic Errors and Bugs

• Can you think of any use case in which the
  code does not behave as intended?
• Can you think of any inputs or external events
  that could break the code?

Error Handling and Logging

• Is error handling done the correct way?
• Should any logging or debugging information
  be added or removed?
• Are error messages user-friendly?
• Are there enough log events and are they
  written in a way that allows for easy
  debugging?

Maintainability

• Is the code easy to read?
• Is the code not repeated (DRY Principle)?
• Is the code method/class not too long?

Dependencies

• Were updates to documentation, configuration, or readme files made as required by this change?
• Are there any potential impacts on other parts of the system or backward compatibility?

Security

• Does the code introduce any security vulnerabilities?

Performance

• Do you think this code change decreases
  system performance?
• Do you see any potential to improve the
  performance of the code significantly?

Testing and Testability

• Is the code testable?
• Have automated tests been added, or have related ones been updated to cover the change?
  • For changes to mutable state
• Do tests reasonably cover the code change (unit/integration/system tests)?
  • Line Coverage
  • Region Coverage
  • Branch Coverage
• Are there some test cases, input or edge cases
  that should be tested in addition?

Readability

• Is the code easy to understand?
• Which parts were confusing to you and why?
• Can the readability of the code be improved by
  smaller methods?
• Can the readability of the code be improved by
  different function, method or variable names?
• Is the code located in the right
  file/folder/package?
• Do you think certain methods should be
  restructured to have a more intuitive control
  flow?
• Is the data flow understandable?
• Are there redundant or outdated comments?
• Could some comments convey the message
  better?
• Would more comments make the code more
  understandable?
• Could some comments be removed by making the code itself more readable?
• Is there any commented-out code?

Documentation

• Is there sufficient documentation?
• Is the ReadMe.md file up to date?

Best Practices

• Follow Single Responsibility principle?
• Are different errors handled correctly?
• Are errors and warnings logged?
• Magic values avoided?
• No unnecessary comments?
• Minimal nesting used?

Experts' Opinion

• Do you think a specific expert, like a security
  expert or a usability expert, should look over
  the code before it can be accepted?
• Will this code change impact different teams, and should they review the change as well?

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[github-actions]

Benchmark results:

╔═╡ Size (-%) ╞════════════════╡ All M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════════════╡ All M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════════════╡ All M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════════════╡ All MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════════════╡ All MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════════════╡ All MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════╡ Precompiles M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════╡ Precompiles M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════╡ Precompiles M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞════════╡ Precompiles MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞══════╡ Precompiles MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞════════╡ Precompiles MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞══════════╡ Real life M3B3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞════════╡ Real life M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞══════════╡ Real life M3B3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

╔═╡ Size (-%) ╞══════════╡ Real life MzB3 ╞═╗
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Cycles (-%) ╞════════╡ Real life MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╠═╡ Ergs (-%) ╞══════════╡ Real life MzB3 ╞═╣
║ Mean                                0.000 ║
║ Best                                0.000 ║
║ Worst                               0.000 ║
║ Total                               0.000 ║
╚═══════════════════════════════════════════╝

[github-actions]

Lit Tests Results

     3 files  ±0       4 suites  ±0   6m 55s ⏱️ +11s
49 113 tests ±0  17 505 ✅ ±0  31 608 💤 ±0  0 ❌ ±0 
69 580 runs  ±0  17 924 ✅ ±0  51 656 💤 ±0  0 ❌ ±0 

Results for commit 81db40d. ± Comparison against base commit 38bb8fb.

♻️ This comment has been updated with latest results.

[FlashSheridan]

There’s an engineering policy question for Dmitry and his team: Are they expected to format (and check) locally code which they’ve touched before pushing, and/or are they willing to accept a certain amount of inconvenience from GitHub Action failures? The local testing I’ve been able to do (see previous comments) suggests that this won’t be an inordinate amount, but may be non-trivial.
A similar issue for clang-tidy had more GitHub Action failures than I’d hoped for, but didn’t elicit any objections that I’m aware of.

[akiramenai]

@FlashSheridan if I got the question right, we are ok to have a failure because of clang-format on CI. It's much better than merge an unformatted patch and then have irrelevant formatting changes committed with something else. We had a lot of that in the past, so I hope this PR will prevent such things in future.

[FlashSheridan]

@FlashSheridan if I got the question right, we are ok to have a failure because of clang-format on CI. It's much better than …

Agreed, and anyway that‘s an engineering policy decision which QA would be wise to stay out of. I was hoping for a third way, though, where an engineer could be sure before committing that his patch was properly formatted. (E.g. with a Git hook exactly matching CI, because it used the same script.) I think my technique above improves the odds, but I don’t know whether it properly handles configuration.

(Sorry for the slow response — taking PTO for more revisions for Logique et Analyse.)

[hedgar2017]

LGTM!
Let's just prevent false-positives if possible, sir.

[akiramenai]

@antonbaliasnikov what prevents us from squashing and merging?

[antonbaliasnikov]

@antonbaliasnikov what prevents us from squashing and merging?

@akiramenai , nothing. I was just waiting for the final confirmation that we're ok with the current implementation and behavior after reviews. I'm rebasing and merging then.

[akiramenai]

You got 2 LGTMs. If you think that we might overlooked something particular, let's discuss the concern. Otherwise, let's get it merged.