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.

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 test for multi GCC, CLang and intel #202

Closed
flagarde opened this issue Nov 27, 2022 · 23 comments
Closed

Add test for multi GCC, CLang and intel #202

flagarde opened this issue Nov 27, 2022 · 23 comments

Comments

@flagarde
Copy link
Collaborator

Hello,

I have created a workflow to test the library on many compiler version etc ... I have tried to push down the GCC and CLang required version. Please see https://github.com/flagarde/cpp-terminal/actions/workflows/ubuntu.yml

Do you like to have a PR on this ? It would solve some of the #182

@MCWertGaming
Copy link
Collaborator

hey! Sorry for not working on this library anymore in the last weeks, I got busy with a different project. Would be cool to have a PR though!

I have a bit more time in the next weeks so I'm thinking about moving this a bit forward again, we can discuss the future of this project if you are interested. I would like to avoid having multiple versions.

@flagarde
Copy link
Collaborator Author

No problem :) I was busy too
what do you mean by multiple versions ?

@MCWertGaming
Copy link
Collaborator

I mean that I'd like to prevent forks when possible. If want to, we could setup a discord call or something to discuss the Future of this project. It has been a while since I have heard from @certik as well, idk how the genereall plan is, especially since it's part of Xeus, the jupyter notebook kernel and also LFortran (while both are using out dated versions or their own in tree version)

@flagarde
Copy link
Collaborator Author

Yes I agree it would be nice to have less fork possible.
I have succeeded to change the code to make it compile on gcc4.6 and clang3.5. This just supress some part of c++ language not very necessary lik auto ans so on... It is able to compile on c++11 too without too much change. I think it's nice to lower a bit the requirement especially as we where no using very powerfull feature from c++14 and c++17 just some sweet syntaxe from c++14 mainly.. What the minimum version of GCC you want to be compatible with ?

@flagarde
Copy link
Collaborator Author

flagarde commented Nov 27, 2022

I'm modifing in my fork the commits are ugly but I think you could merge them together to start with all the compilers working and then to see which feature of c++ we really need and supress the gcc/clang version incompatible with this. What do you think?

I have create a ci depot you could fork to have your own and run on it because I can not guarantee good versionning etc : https://github.com/flagarde/ci

The biggest feature for me missing before gcc4.7 is Non-static data member initializers. I like this feature. gcc4.7.4 is from 2014

gcc4.5 don't have nullptr so it start to be very bad ;-)

@flagarde
Copy link
Collaborator Author

I would propose this strategie :

  1. Add as much as workflows possible. I can provide ones for windows msys macos linux etc and let the actual one with _old.yml names. (I will send a PR soon for this)
  2. Activate all the warnings for all the compilers.
  3. Make all the compilers happy with c++17
  4. Lower down slowly from c++17 to c++11 and if we seen some nice c++ feature have to disappear (something we don't want to loose, than stop to these versions of compiler and mark them on marble)

What do you think ?

@flagarde
Copy link
Collaborator Author

@MCWertGaming @certik Please have a view on #203. As soon as this is pushed it would be easier to make all the c++17 pass and then move down to c++14 and c++11 if required. c++17 and even c++14 looks very high for what cpp-terminal code has. It's possibe to use only c++11 just by using old syntax from c++1 like no auto keyworks, no grouped namespace like namespace Term::GG{ but namespace Term{ namespace GG{ etc... Nothing very needed I think. What do you think about trying to stay compatible to c++11 and only move up if we really need a feature on c++14 or c++17 ?

@certik
Copy link
Collaborator

certik commented Nov 28, 2022

Yes, I think we should try to be compatible with C++11. I think you can use auto in C++11, but maybe not in all circumstances?

@flagarde
Copy link
Collaborator Author

maybe not with all compilers :(

@CharlesBunders
Copy link

I would like to help with what I can.

@flagarde
Copy link
Collaborator Author

flagarde commented Nov 29, 2022

I succeed to move down to c++14 and soon to c++11 to all compilers I will to some PR. First move is to push #203 it should fix teh problem of #201

@flagarde flagarde mentioned this issue Nov 29, 2022
@flagarde
Copy link
Collaborator Author

@CharlesBunders Fix is merged. You can have a try

@CharlesBunders
Copy link

@flagarde Looks good.

@MCWertGaming
Copy link
Collaborator

Yeah, merged most things now, sorry for the wait! I guess that the best thing for cpp-terminal is to be as compatible as possible to all environments ^^

@MCWertGaming
Copy link
Collaborator

It seems though that github gives us a rate limit with all those CI tasks, I hope that this won't be an issue for us.

@flagarde
Copy link
Collaborator Author

flagarde commented Dec 6, 2022

Yeah, merged most things now, sorry for the wait! I guess that the best thing for cpp-terminal is to be as compatible as possible to all environments ^^

Yes, at least it seems we can be compatible with a lot without torturing to much the code...And now there is CI to test all the version. So now it is possible to see which version is lost using such or such feature and decide accordingly

@flagarde
Copy link
Collaborator Author

flagarde commented Dec 6, 2022

It seems though that github gives us a rate limit with all those CI tasks, I hope that this won't be an issue for us.

Strange, i never seen this problem before playing with them... If so we could comment some of them or trigger them on demand

@MCWertGaming
Copy link
Collaborator

Sounds good. For the github actions there is no action needed yet, we'll see how it's performing at the next PRs ^^

@MCWertGaming
Copy link
Collaborator

Hey @flagarde are there more things you would like to contribute?

@flagarde
Copy link
Collaborator Author

@MCWertGaming I'm working on activating all the warnings for compilers, and an option to turn them into errors when we are free from most of them... Then there is some changes I would like to discuss with you and @certik . Then I could from time to time do some PR for the issues already there.

@MCWertGaming
Copy link
Collaborator

@MCWertGaming I'm working on activating all the warnings for compilers, and an option to turn them into errors when we are free from most of them... Then there is some changes I would like to discuss with you and @certik . Then I could from time to time do some PR for the issues already there.

Sure! Feel free to open a PR with all errors, we can look into fixing them then together.

@flagarde
Copy link
Collaborator Author

flagarde commented Jan 2, 2023

@MCWertGaming this could be closed?

@MCWertGaming
Copy link
Collaborator

Yeah, thank you all for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants