-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add old, still supported versions in CI matrix #692
Add old, still supported versions in CI matrix #692
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #692 +/- ##
==========================================
+ Coverage 94.48% 94.51% +0.02%
==========================================
Files 48 48
Lines 7091 7177 +86
==========================================
+ Hits 6700 6783 +83
- Misses 391 394 +3
... and 3 files with indirect coverage changes
|
Can you update the release notes document with the supported/tested versions. I think right now it says 5 to 17. |
|
||
# Оld, still supported versions | ||
|
||
- name: ubu20-gcc9-runtime7 |
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.
It's indeed good that we still support these versions. However, should we spend efforts in maintaining such old versions of Clad?
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 think we should support 10 versions back only. The support generally has meant to compile with that version and not much else since we are using high-level clang API that produce consistent results.
min fact this pull request restricts the versions we support because so far we claim we support clang5 ;)
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.
Yes, if the support comes down to only having the corresponding fix in Compatibility.h, then 10 versions back is fine. However, if at some point this starts to interfere with us and take more time, then more versions may be dropped.
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.
@vgvassilev @alexander-penev Thank you for the explanation.
clang-tidy review says "All clean, LGTM! 👍" |
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.
LGTM!
@alexander-penev can you apply clang-format to the diff (as suggested by the CI). |
0c90bda
to
d906af6
Compare
clang-tidy review says "All clean, LGTM! 👍" |
No description provided.