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.

Sign up for GitHub

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

Jump to bottom

Enabled C++ 20 if it is available. #239

Closed
wants to merge 1 commit into from
Closed

Enabled C++ 20 if it is available. #239

wants to merge 1 commit into from

Conversation

KOLANICH
Copy link
Contributor

No description provided.

@SRombauts SRombauts self-assigned this Dec 24, 2019
@coveralls
Copy link

coveralls commented Dec 25, 2019 •
edited
Loading

Coverage Status

Coverage remained the same at 100.0% when pulling cb655a9 on KOLANICH:cpp20 into ffc1a3e on SRombauts:master.

@KOLANICH
Copy link
Contributor Author

I have no idea why https://travis-ci.org/SRombauts/SQLiteCpp/jobs/629439562 fails

@KOLANICH KOLANICH changed the base branch from master to sqlitecpp-3.x December 26, 2019 20:10
@SRombauts SRombauts force-pushed the sqlitecpp-3.x branch 23 times, most recently from 757dac8 to 3db494e Compare January 1, 2020 23:01
@SRombauts SRombauts closed this Jan 4, 2020
@KOLANICH
Copy link
Contributor Author

KOLANICH commented Jan 4, 2020

????

@SRombauts
Copy link
Owner

Ah ah, sorry, I merged the branch 3.x into master and deleted it an hour ago, which seems to to have close the PR automatically: please retarget it to master.

@SRombauts SRombauts reopened this Jan 9, 2020
@KOLANICH KOLANICH force-pushed the cpp20 branch 2 times, most recently from 9a62ad8 to 38cbef7 Compare January 13, 2020 09:58
@KOLANICH KOLANICH changed the base branch from sqlitecpp-3.x to master January 13, 2020 22:02
@SRombauts
Copy link
Owner

Hello, so in this case, I don't think it would be a good idea to have a library try to use C++ latest standard.
I would rather have a minimum/required version (C++11 right now) and the application using it deciding what is best.

What would be interesting is to add more test cases to the Github Actions to test with latest compilers and latest C++ standard in CI

@KOLANICH
Copy link
Contributor Author

  1. It is optional. I mean it has to be explicitly enabled. Of course I can provide the needed CMake option via CLI, butnit has some logic dependent on CMake version.
  2. If you still dislike it, feel free to close the PR

SRombauts
SRombauts requested changes Jan 14, 2020
View reviewed changes
Copy link
Owner

@SRombauts SRombauts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like where it is going, please have a look at the 2 comments I left

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@KOLANICH KOLANICH force-pushed the cpp20 branch 2 times, most recently from 2861326 to 68b9e01 Compare January 14, 2020 12:36
@KOLANICH
Added an option to enable C++ 20 if it is available.
cb655a9 
Had to remove CMAKE_CXX_STANDARD_REQUIRED because it would have caused requiring too high standard on opportunistic upgrade
@SRombauts SRombauts mentioned this pull request Jan 17, 2020
Closed
This was referenced Jan 30, 2020
Closed
Merged
@emmenlau
Copy link
Contributor

@KOLANICH would #269 also solve your needs? I think it requires less changes but gives you the same flexibility. Is that good?

@SRombauts
Copy link
Owner

dropped in favor of #269

@SRombauts SRombauts closed this Jan 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SRombauts SRombauts SRombauts requested changes

Assignees

@SRombauts SRombauts

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@KOLANICH @coveralls @SRombauts @emmenlau