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

MueLu: Code on 'develop' fails clang-format check breaking every PR starting 2024-10-10 #13524

Open
bartlettroscoe opened this issue Oct 11, 2024 · 6 comments · Fixed by #13527
Labels
PA: Framework Issues that fall under the Trilinos Framework Product Area pkg: MueLu type: bug The primary issue is a bug in Trilinos code or tests

Comments

@bartlettroscoe
Copy link
Member

Bug Report

Somehow, MueLu code on 'develop' is failing the clang-format check for PRs that don't touch MueLu. For example, for the PR:

which does not touch any clang-format settings or file under packages/muelu, the clang-format GHA check fails:

showing the diff:

diff --git a/packages/muelu/test/unit_tests/IntrepidPCoarsenFactory.cpp b/packages/muelu/test/unit_tests/IntrepidPCoarsenFactory.cpp
index 6a1082a1..c324c1cc 100644
--- a/packages/muelu/test/unit_tests/IntrepidPCoarsenFactory.cpp
+++ b/packages/muelu/test/unit_tests/IntrepidPCoarsenFactory.cpp
@@ -48,7 +48,7 @@ namespace MueLuTests {
 #ifndef TEST_MORE_COMBINATIONS
 static const int MAX_LINE_DEGREE = (Intrepid2::Parameters::MaxOrder < 10) ? Intrepid2::Parameters::MaxOrder : 10;
 static const int MAX_QUAD_DEGREE = (Intrepid2::Parameters::MaxOrder < 10) ? Intrepid2::Parameters::MaxOrder : 10;
-static const int MAX_HEX_DEGREE  = (Intrepid2::Parameters::MaxOrder < 4) ? Intrepid2::Parameters::MaxOrder  :  4;
+static const int MAX_HEX_DEGREE  = (Intrepid2::Parameters::MaxOrder < 4) ? Intrepid2::Parameters::MaxOrder : 4;
 static const int MAX_RANK_COUNT  = 4;
 #else
 static const int MAX_LINE_DEGREE = Intrepid2::Parameters::MaxOrder;

As shown here:

all of these checks have failed for every PR for the last 2 hours impacting the PRs:

How did this happen?

@bartlettroscoe bartlettroscoe added pkg: MueLu type: bug The primary issue is a bug in Trilinos code or tests labels Oct 11, 2024
Copy link

Automatic mention of the @trilinos/muelu team

1 similar comment
Copy link

Automatic mention of the @trilinos/muelu team

@bartlettroscoe bartlettroscoe pinned this issue Oct 11, 2024
@bartlettroscoe bartlettroscoe changed the title MueLu: Code on 'develop' fails clang-format check MueLu: Code on 'develop' fails clang-format check breaking every PR starting 2024-10-10 Oct 11, 2024
@bartlettroscoe
Copy link
Member Author

Looking at the commit history for the file packages/muelu/test/unit_tests/IntrepidPCoarsenFactory.cpp, it looks like the commit 6a817df caused this.

How did that commit get onto the 'develop' branch? @sebrowne?

@bartlettroscoe bartlettroscoe added the PA: Framework Issues that fall under the Trilinos Framework Product Area label Oct 11, 2024
@cgcgcg
Copy link
Contributor

cgcgcg commented Oct 11, 2024

The problem is that the clang check has not been marked as required. Fortunately this also means that it won't block any PRs from merging now.

@bartlettroscoe
Copy link
Member Author

bartlettroscoe commented Oct 11, 2024

The problem is that the clang check has not been marked as required. Fortunately this also means that it won't block any PRs from merging now.

They why are we getting emails about this? Non-required checks should not be triggering notifications.

Update: I guess I am just confused why the Trilinos GH repo would be configured to send out failure notifications for failing GHA checks that the PR author can do nothing to fix. Is this just not conditioning developers to ignore the GHA failures?

@bartlettroscoe bartlettroscoe unpinned this issue Oct 11, 2024
@cgcgcg cgcgcg linked a pull request Oct 14, 2024 that will close this issue
@cgcgcg
Copy link
Contributor

cgcgcg commented Nov 1, 2024

@trilinos/framework Can we switch the clang-format check to "Required" to avoid this in the future?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PA: Framework Issues that fall under the Trilinos Framework Product Area pkg: MueLu type: bug The primary issue is a bug in Trilinos code or tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants