Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the issue with using available clang-format version in format #288

Merged
merged 2 commits into from
Apr 29, 2022

Conversation

maliasadi
Copy link
Member

make format (and make format-cpp) now accepts the version number in case clang-format is not installed or not satisfying the default version -- that is currently 12. This update avoids inconsistencies between different clang-format aliases and eases trying different versions when needed.

@maliasadi maliasadi requested review from mlxd and chaeyeunpark April 28, 2022 19:23
@github-actions
Copy link
Contributor

Hello. You may have forgotten to update the changelog!
Please edit .github/CHANGELOG.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@github-actions
Copy link
Contributor

github-actions bot commented Apr 28, 2022

Test Report (C++) on Ubuntu

           1 files  ±0             1 suites  ±0   0s ⏱️ ±0s
       711 tests ±0         711 ✔️ ±0  0 💤 ±0  0 ±0 
228 386 runs  ±0  228 386 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit 3a83e74. ± Comparison against base commit 9b585ec.

♻️ This comment has been updated with latest results.

Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @maliasadi

I re-enabled the failed tests. Once They pass I'll approve

@codecov
Copy link

codecov bot commented Apr 28, 2022

Codecov Report

Merging #288 (3a83e74) into master (9b585ec) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #288   +/-   ##
=======================================
  Coverage   98.86%   98.86%           
=======================================
  Files          27       27           
  Lines        2913     2913           
=======================================
  Hits         2880     2880           
  Misses         33       33           
Impacted Files Coverage Δ
pennylane_lightning/_version.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b585ec...3a83e74. Read the comment docs.

@chaeyeunpark
Copy link
Contributor

@maliasadi Thanks for the change (and increase the readability of format)! In fact, I wrote some part of version checking as I had found that the CI does not accept locally formatted C++ codes because of the version difference. Still, I thought it should be refactored as it is hard to read the code there. Thanks for doing this.

Copy link
Member

@mlxd mlxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

@maliasadi maliasadi merged commit 6537614 into master Apr 29, 2022
@maliasadi maliasadi deleted the up_cpp_format branch April 29, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants