ARMmbed / mbed-tools

⚠️ Beta Status: New command line tooling for Mbed OS
Apache License 2.0
45 stars 30 forks source link

config: Do not prepend definitions with `-D` #299

Closed LDong-Arm closed 3 years ago

LDong-Arm commented 3 years ago

Description

The CMake list MBED_CONFIG_DEFINITIONS in mbed_config.cmake generated by mbed-tools configure has each item prepended with -D. Mbed OS passes this list to target_compile_definitions() which supports macros with or without -D.

Dropping -D from MBED_CONFIG_DEFINITIONS makes the style consistent with MBED_TARGET_DEFINITIONS and makes it easier for a library or application to check if a given macro is on the list.

Test Coverage

LDong-Arm commented 3 years ago

I can't ever add @ARMmbed/mbed-os-core as a reviewer in this repo. Maybe the team alias needs to be given permission to this repo?

0xc0170 commented 3 years ago

The team needs to be collaborator (invite the team to this repo), the team is visible to this repository as reviewers once invited.

codecov[bot] commented 3 years ago

Codecov Report

Merging #299 (57d2e31) into master (57d9e7b) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #299   +/-   ##
=======================================
  Coverage   97.08%   97.08%           
=======================================
  Files          92       92           
  Lines        2782     2782           
=======================================
  Hits         2701     2701           
  Misses         81       81           
rwalton-arm commented 3 years ago

The team needs to be collaborator (invite the team to this repo), the team is visible to this repository as reviewers once invited.

I've added mbed-os-core as a collaborator. Maybe it works now?

LDong-Arm commented 3 years ago

@rwalton-arm Thanks, it works now!