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.

Sign up for GitHub

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

Jump to bottom

feat(ci): change pre-commit clang-format to run in ci [skip pre-commit.ci] #1373

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat(ci): change pre-commit clang-format to run in ci [skip pre-commit.ci] #1373

wants to merge 9 commits into from

Conversation

wdconinc
Copy link
Contributor

@wdconinc wdconinc commented Apr 8, 2024

Briefly, what does this PR introduce?

We don't let the CI pre-commit job run automatic clang-format and cmake-format fixes. This PR changes that behavior for clang-format, by updating the clang-format pre-commit hook.

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No changes.

Does this PR change default behavior?

Yes, it might lead to developers seeing their code reformatted automatically.

@wdconinc
Copy link
Contributor Author

wdconinc commented Apr 8, 2024

pre-commit.ci autofix

@github-actions github-actions bot added topic: documentation Improvements or additions to documentation topic: calorimetry relates to calorimetry topic: tracking Relates to tracking reconstruction topic: PID Relates to PID reconstruction topic: far-forward Far forward reconstruction topic: far-backward Reconstruction related to far backward detectors topic: infrastructure topic: barrel topic: forward topic: backward topic: jets topic: digitization labels Apr 8, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Apr 8, 2024 •
edited
Loading

Capybara summary for PR 1373

@wdconinc wdconinc requested a review from veprbl April 8, 2024 23:14
@wdconinc
Copy link
Contributor Author

After squash merge, we should add the commit to .git-blame-ignore-revs, https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view.

veprbl
veprbl reviewed Apr 11, 2024
View reviewed changes
docs/arches_flags.json Outdated Show resolved Hide resolved
src/algorithms/calorimetry/CalorimeterHitDigi.cc Outdated Show resolved Hide resolved
.clang-format Show resolved Hide resolved
@wdconinc wdconinc requested a review from veprbl April 12, 2024 14:41
@wdconinc
Copy link
Contributor Author

clang-tidy error https://github.com/eic/EICrecon/actions/runs/8663802856/job/23763825088#step:9:120 is caused by the yaml file containing segments like:

- BuildDirectory: /home/runner/work/EICrecon/EICrecon/build/src/utilities/eicrecon
  DiagnosticMessage:
    FileOffset: 0
    FilePath: ''
    Message: macro replacement list should be enclosed in parentheses
    Replacements:
    - FilePath: ''
      Length: 0
      Offset: 15074
      ReplacementText: (
    - FilePath: ''
      Length: 0
      Offset: 15088
      ReplacementText: )
  DiagnosticName: bugprone-macro-parentheses
  Level: Warning

where the FilePath is empty. We should fix this, but the byte offset is not letting me figure out where the error is...

github-actions[bot]
github-actions bot previously requested changes Apr 12, 2024
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ Clang-Tidy found issue(s) with the introduced code (1/1)

src/algorithms/reco/MatchClusters.cc Show resolved Hide resolved
src/benchmarks/reconstruction/tracking_efficiency/TrackingEfficiency_processor.cc Show resolved Hide resolved
src/algorithms/digi/PhotoMultiplierHitDigi.cc Show resolved Hide resolved
src/algorithms/tracking/TrackPropagation.cc Show resolved Hide resolved
src/algorithms/tracking/TrackPropagation.cc Show resolved Hide resolved
src/benchmarks/reconstruction/tof_efficiency/TofEfficiency_processor.cc Show resolved Hide resolved
src/algorithms/reco/ScatteredElectronsEMinusPz.cc Show resolved Hide resolved
@wdconinc wdconinc changed the title feat(ci): change pre-commit clang-format and run in ci feat(ci): change pre-commit clang-format to run in ci [skip pre-commit.ci] Apr 12, 2024
@wdconinc wdconinc dismissed github-actions[bot]’s stale review April 12, 2024 21:14

Rebased, no changes in these files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@veprbl veprbl Awaiting requested review from veprbl

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
topic: backward topic: barrel topic: calorimetry relates to calorimetry topic: digitization topic: documentation Improvements or additions to documentation topic: far-backward Reconstruction related to far backward detectors topic: far-forward Far forward reconstruction topic: forward topic: infrastructure topic: jets topic: PID Relates to PID reconstruction topic: tracking Relates to tracking reconstruction
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@wdconinc @veprbl