-
Notifications
You must be signed in to change notification settings - Fork 118
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
Require C++17 #1893
Require C++17 #1893
Conversation
When you bump version of nrn, you can in the same time bump the one for InterViews. Normally it's already c++17 ready due to: neuronsimulator/iv#39 |
@olupton: Does any of our CI (images) from nrn-build-ci include flex < 2.6? Edit: Hmm..Ok, I see the failing CIs. |
macOS-10.15 (regular CI + nrn-build-ci) does: https://github.com/neuronsimulator/nrn/runs/7128845752#step:14:216 |
Codecov Report
@@ Coverage Diff @@
## master #1893 +/- ##
==========================================
- Coverage 47.17% 47.13% -0.04%
==========================================
Files 543 543
Lines 112912 112912
==========================================
- Hits 53264 53219 -45
- Misses 59648 59693 +45
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
✔️ 18456416f27b8e0d0bf70b8c9c772d1b5b21c168 -> Azure artifacts URL |
✔️ 58e8ec65c83dd459a96d8ac564460ce802cad037 -> Azure artifacts URL |
✔️ dfdd0a246f99810402323806b92767293a8bdb6b -> Azure artifacts URL |
✔️ ae699d5 -> Azure artifacts URL |
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
✔️ 12530f3 -> Azure artifacts URL |
This reverts commit dfb7e7b.
✔️ 655d2b1 -> Azure artifacts URL |
LGTM, thanks for the finishing touches @alexsavulescu. |
I checked this URL in https://bbpgitlab.epfl.ch/hpc/personal/neuron-gpu-wheel-testing/-/pipelines/63923 using the |
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
Let's see what breaks...
Based on https://en.cppreference.com/w/cpp/compiler_support/17, this will correspond to requiring ~GCC7 or ~Clang5.
TODO:
flex
2.6+ is required (older versions emit theregister
keyword)