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

Update/cleanup cmake files #579

Merged

Conversation

mlasch
Copy link
Contributor

@mlasch mlasch commented Apr 6, 2021

This is a pre-work/cleanup to make the CoAP block-size configurable during build time (via cmake variable) in a future PR.

  • Replace redundant WITH_LOG since it was only rarely used and we have LWM2M_WITH_LOG.
  • Replace add_definitions with modern alternatives.
  • Set a common minimal required cmake version.

I bumped the cmake version in all files to 3.13 to support all required features and to be consistent (3.13 is currently available in Debian stable, so still quite conservative). Also tried to make the formatting a bit more homogeneous.

@@ -7,15 +7,18 @@ include(${CMAKE_CURRENT_LIST_DIR}/../coap/coap.cmake)
include(${CMAKE_CURRENT_LIST_DIR}/../data/data.cmake)
include(${CMAKE_CURRENT_LIST_DIR}/../examples/shared/shared.cmake)

add_definitions(-DLWM2M_CLIENT_MODE -DLWM2M_SUPPORT_TLV -DLWM2M_SUPPORT_JSON)
add_compile_definitions(LWM2M_CLIENT_MODE)
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the idea of using multiple lines.

@rettichschnidi
Copy link
Contributor

FYI: This will conflict with PR #576. One of us will have to rebase.

@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 2.8)
cmake_minimum_required(VERSION 3.13)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cmake_minimum_required(VERSION 3.13)
cmake_minimum_required (VERSION 3.13)

Copy link

Choose a reason for hiding this comment

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

Why the extra space?

Copy link
Contributor

Choose a reason for hiding this comment

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

For consistency reasons - it has this space everywhere else too. But really not important, which is why have approved the PR anyway.

@sbernard31
Copy link
Contributor

Could you rebase on master ? 🙏

mlasch added 2 commits April 7, 2021 13:00
 - Replace redundant WITH_LOG since it was only rarely used and we have LWM2M_WITH_LOG.
 - Replace add_definitions with modern alternatives.
 - Set a common minimal required cmake version.

Signed-off-by: Marc Lasch <[email protected]>
@mlasch mlasch force-pushed the gardena/ml/cleanup-cmake-definitions branch from 4055361 to 8f10a8b Compare April 7, 2021 11:01
@sbernard31 sbernard31 merged commit 8f10a8b into eclipse-wakaama:master Apr 7, 2021
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.

4 participants