-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 C++ standards to CI config (Linux) #1992
Conversation
c1494bc
to
a4cbd92
Compare
standard: 20 | ||
|
||
### gcc 8 | ||
# has c++14 as default mode |
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.
Doesn't mean someone won't use c++11 on gcc-8.
If resources aren't a constrain I wouldn't exclude it from checks.
Same comment for other excludes like this one.
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 was just impressed by the number of configurations without any filtering.
Maybe I over-filtered them.
But I don't know if they make any sense, for example, is it possible that c++11 on gcc-8 fails while c++11 on gcc-4.8 doesn't? It can be possible of course, because of some bugs, but if we are hunting for bugs here then this config should have a huge number of configurations.
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 see your point.
However following this though, standard shouldn't be a matrix parameter.
Check default on each compiler and have separate matrix for all cpp standards tested on newest compilers only.
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.
Ok, this is a good suggestion.
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.
But I think it will be better to create a new PR with the described logic because the logic here and the logic you proposed differ. I can create it, or maybe you want to do this?
A quick note here in case this is preferred over #1999: You may want to consider disabling C++20 support on |
@jgopel hmm... but C++20 on gcc-8 is disabled. I'm closing this PR. #1999 is better because it has additional fixes. I think the most important thing in this PR is the idea proposed by @darklukee that standard shouldn't be a matrix parameter Following this idea I would make a configuration with several jobs inside, like:
Where the
Off-top: what a shame that GitHub Action doesn't support YAML anchors. |
I totally missed the additional blocks after the gcc-4.8 block - I stand corrected |
For Linux workflow, I added several C++ standards with excluding unsupported/excess standard for each presented compiler.
This is related to the issue #1987.
Also, I fixed 1 compile warning (-Werror,-Wsign-conversion) for clang++ with C++17 standard, otherwise, CI build fails.