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

Make it possible to build with c++20 and switch to new Key4hep build action #114

Merged
merged 3 commits into from
Feb 5, 2024
Merged

Make it possible to build with c++20 and switch to new Key4hep build action #114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fa02d8e
Make it possible to build with c++20
tmadlener Feb 1, 2024
431f38d
Run new key4hep workflows on PR commits
tmadlener Feb 1, 2024
5c4c781
Remove old key4hep workflow
tmadlener Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/key4hep-build.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ name: Key4hep build
on:
push:
workflow_dispatch:
pull_request:
Copy link
Member

@jmcarcell jmcarcell Feb 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This will get overwritten if it's not changed in key4hep-actions. Why is this needed? With the push: they run whenever there is a push in a branch, in a PR or not, pull_request: will duplicate the jobs for a PR since they will run in the PR and after push meaning it takes longer to find workers and for all the jobs to run

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without the pull_request they will not report back to the PR. The push hook only runs them on the branch (which is in my fork). Maybe push: master and pull_request: is the way to go?

Copy link
Member

Choose a reason for hiding this comment

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

Yes I think that's it. The repos have to have the same name for the default branch though but in this case it's true for key4hep

Copy link
Member

Choose a reason for hiding this comment

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

Ah no I just remembered why I didn't go for that. With that, the workflows don't run in your workflow when you push to a different branch other than the default one and some people don't like that. I couldn't find a way of having it

  • Run once in PRs
  • Run also when pushing to a fork
  • Report in the PR page
    The closest is the first two without the third one with only push:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. But the one that we definitely need is the reporting, right? Otherwise we cannot use auto-merge with required checks.

I also didn't find a way to restrict the push to only the main repository, which I guess would be nice, because then there would be no duplicate reporting of failures.

tmadlener marked this conversation as resolved.
Show resolved Hide resolved

jobs:
build:
Expand Down
43 changes: 0 additions & 43 deletions .github/workflows/test.yml
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ set(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE CACHE BOOL "RPATH USE LINK PATH")

#--- Declare C++ Standard -----------------------------------------------------
set(CMAKE_CXX_STANDARD 17 CACHE STRING "")
if(NOT CMAKE_CXX_STANDARD MATCHES "17")
if(NOT CMAKE_CXX_STANDARD MATCHES "17|20")
message(FATAL_ERROR "Unsupported C++ standard: ${CMAKE_CXX_STANDARD}")
endif()
message (STATUS "C++ standard: ${CMAKE_CXX_STANDARD}")
Expand Down
Loading