-
Notifications
You must be signed in to change notification settings - Fork 24
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
[ci] Add latest cling versions(LLVM 16, 18) #333
base: main
Are you sure you want to change the base?
Conversation
4bea50d
to
ad7fbd7
Compare
@aaronj0 You'll need to delete the jobs related to cling and llvm 13, and just keep the job related to llvm 18. If you don't the llvm cache will be overwhelmed. |
.github/workflows/ci.yml
Outdated
cppyy: Off | ||
- name: osx14-arm-clang-clang-repl-19 | ||
- name: osx14-arm-clang-repl-19 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the exception of Windows the names of jobs are currently
'operation system'-architechture-compiler-'cling or clang-repl version'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer easier readability. Compiler backend are either clang-repl or cling. 'clang-clang-repl' does not make sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaronj0 That's fine to do but we need to be consistent, so you've need to drop the msvc and gcc12 from the other jobs for consistency.
.github/workflows/ci.yml
Outdated
os: windows-2022 | ||
compiler: msvc | ||
clang-runtime: '13' | ||
clang-runtime: '16' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the clang llvm 16 jobs as this will overwhelm the llvm cache. We need to just keep the latest version of llvm with respect to cling.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need both. Cling differs with the LLVM-18 upgrade and ROOT requires this check to be made to support releases >6.32 . I would rather drop clang-repl16
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aaronj0 We need to drop one set of jobs. If you think it would be better to drop clang-repl-16 then I am ok with that. We can continue to support those jobs in the other repos for now since they don't have any Windows builds filling up the cache.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #333 +/- ##
==========================================
- Coverage 74.27% 73.83% -0.44%
==========================================
Files 8 8
Lines 3188 3188
==========================================
- Hits 2368 2354 -14
- Misses 820 834 +14
... and 1 file with indirect coverage changes
|
d289b6a
to
6075374
Compare
clang-tidy review says "All clean, LGTM! 👍" |
clang-tidy review says "All clean, LGTM! 👍" |
clang-tidy review says "All clean, LGTM! 👍" |
91ba92f
to
435816e
Compare
No description provided.