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 clang-format workflow #251

Merged

Conversation

mcbarton
Copy link
Collaborator

@mcbarton mcbarton commented Feb 4, 2025

Description

Please include a summary of changes, motivation and context for this PR.

The clang format workflow is broken because it tries to get a git-clang-format by wgetting it. Git-clang-format is not used as far as I can tell from the workflow, so should be safe to just remove the 'Download git-clang-format' section of the workflow.

Fixes # (issue)

Type of change

Please tick all options which are relevant.

  • Bug fix
  • New feature
  • Added/removed dependencies
  • Required documentation updates

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.72%. Comparing base (5076c46) to head (c183b9b).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #251   +/-   ##
=======================================
  Coverage   80.72%   80.72%           
=======================================
  Files          19       19           
  Lines         970      970           
  Branches       93       93           
=======================================
  Hits          783      783           
  Misses        187      187           
Files with missing lines Coverage Δ
src/main.cpp 40.67% <ø> (ø)
Files with missing lines Coverage Δ
src/main.cpp 40.67% <ø> (ø)

Copy link
Contributor

github-actions bot commented Feb 4, 2025

clang-tidy review says "All clean, LGTM! 👍"

@anutosh491
Copy link
Collaborator

Looks okay to me but we know the CI would wail for the emscripten build. I'll look into that first and then we can move this in !

@@ -5,7 +5,6 @@
* *
* The full license is in the file LICENSE, distributed with this software. *
****************************************************************************/

Copy link
Collaborator

@anutosh491 anutosh491 Feb 7, 2025

Choose a reason for hiding this comment

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

Not sure why this line was removed.

Copy link
Collaborator

@anutosh491 anutosh491 left a comment

Choose a reason for hiding this comment

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

Looks Okay to me but I've not dealt with clang format work on xeus-cpp in the past so I'll just wait for Vassil or someone else to merge.

Copy link
Contributor

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

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

Lgtm.

@vgvassilev vgvassilev merged commit f054951 into compiler-research:main Feb 7, 2025
16 checks passed
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.

4 participants