Skip to content

build: enable c++17 and use some c++14/17 features#741

Merged
pstorz merged 2 commits intomasterfrom
dev/pstorz/master/c++17
Mar 17, 2021
Merged

build: enable c++17 and use some c++14/17 features#741
pstorz merged 2 commits intomasterfrom
dev/pstorz/master/c++17

Conversation

@pstorz
Copy link
Member

@pstorz pstorz commented Feb 23, 2021

  • Enable c++17 support in cmake
    • Avoid warnings because the "register" keyword was removed in c++17
      • NDMP files generated by rpcgen
      • Python2 header files
      • Windows: glob.cc
    • Use c++14 digit separator (1'000'000)

Please follow these few prerequisites before you hand in a PR

Consider the following chapters in the Bareos Developer Documentation

Add some information

  • Add a small description to the CHANGELOG.md file and refer to your PR using this syntax '[PR #xyz]'
  • Add your name to the AUTHORS file

Keep spirit!

  • Do not be afraid to hand in a PR!

@pstorz pstorz requested a review from arogge February 23, 2021 09:25
@pstorz pstorz force-pushed the dev/pstorz/master/c++17 branch 4 times, most recently from 9984191 to b19062b Compare February 23, 2021 16:38
Copy link
Member

@arogge arogge left a comment

Choose a reason for hiding this comment

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

I have some minor remarks. Overall great work!

Comment on lines 30 to 35
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to keep this? I think it was great for testing, but maybe not for the master branch.
We could do something like that in core/src/compile_tests, so we can make sure string_view actually works.

Most of the interesting C++17 features will require gcc-7 and libstdc++-7 nevertheless, so we won't really need that workaround in the long term.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, I just pushed a commit that removes these parts.

Comment on lines 61 to 62
Copy link
Member

Choose a reason for hiding this comment

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

see above

Copy link
Member Author

Choose a reason for hiding this comment

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

See above

@pstorz pstorz force-pushed the dev/pstorz/master/c++17 branch 6 times, most recently from fe40a5e to bc73302 Compare February 25, 2021 09:32
@pstorz pstorz self-assigned this Feb 25, 2021
@pstorz pstorz force-pushed the dev/pstorz/master/c++17 branch 2 times, most recently from 673f2f6 to 44acf0a Compare March 11, 2021 10:15
pstorz added 2 commits March 17, 2021 11:40
- Enable c++17 support in cmake
- Avoid warnings because the "register" keyword was removed in c++17
  - NDMP files generated by rpcgen
  - Python2 header files
  - Windows: glob.cc
- Use c++14 digit separator (1'000'000)
@pstorz pstorz force-pushed the dev/pstorz/master/c++17 branch from 44acf0a to fd5721d Compare March 17, 2021 10:41
@pstorz pstorz merged commit 85a2c52 into master Mar 17, 2021
@pstorz pstorz deleted the dev/pstorz/master/c++17 branch March 17, 2021 16:21
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

Successfully merging this pull request may close these issues.

2 participants

Comments