Skip to content

configure: Add --profile for profile selection #293

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

Merged
merged 2 commits into from
Jul 2, 2021

Conversation

LDong-Arm
Copy link
Contributor

Description

As mbed-tools compile supports -b, --profile for selecting a build profile, the same should exist for mbed-tools configure as users may want to manually run the build step using cmake in some cases. This PR implements it and supplies a test.

Note: This only affects the build directory - it has no impact on the contents of mbed_config.cmake. To actually build an application with a profile, a user needs to either run mbed-tools compile with --profile <profile>, or manually run cmake with -DCMAKE_BUILD_TYPE=<profile> after mbed-tools configure.

Also add missing test for --profile of mbed-tools compile.

Fixes #262

Test Coverage

  • This change is covered by existing or additional automated tests.
  • Manual testing has been performed (and evidence provided) as automated testing was not feasible.
  • Additional tests are not required for this change (e.g. documentation update).

@LDong-Arm LDong-Arm force-pushed the configure_profile branch from 5d311d9 to 71eea10 Compare June 30, 2021 14:56
@LDong-Arm
Copy link
Contributor Author

I first made the fix locally while working on #292 (--app-config) as they are quite similar, but decided to not include this there as more time was needed to add a test. Now it's done here.

@codecov
Copy link

codecov bot commented Jun 30, 2021

Codecov Report

Merging #293 (923b263) into master (a97be74) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #293   +/-   ##
=======================================
  Coverage   97.08%   97.08%           
=======================================
  Files          92       92           
  Lines        2781     2782    +1     
=======================================
+ Hits         2700     2701    +1     
  Misses         81       81           
Impacted Files Coverage Δ
src/mbed_tools/cli/configure.py 100.00% <100.00%> (ø)

@LDong-Arm
Copy link
Contributor Author

@ARMmbed/mbed-os-core

@rwalton-arm
Copy link
Contributor

Travis timed out. Could you force push to trigger it again?

@LDong-Arm LDong-Arm force-pushed the configure_profile branch from 71eea10 to e1355e2 Compare July 1, 2021 13:15
@LDong-Arm
Copy link
Contributor Author

Travis timed out. Could you force push to trigger it again?

Done. Waiting for CI to finish.

@@ -186,6 +186,29 @@ def test_app_config_used_when_passed(
generate_config.assert_called_once_with(target.upper(), toolchain.upper(), program)
self.assertEqual(program.files.app_config_file, app_config_path)

def test_profile_used_when_passed(
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch.

Copy link
Contributor

@rwalton-arm rwalton-arm left a comment

Choose a reason for hiding this comment

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

LGTM.

LDong-Arm added 2 commits July 2, 2021 10:19
The `compile` subcommand supports `--profile` to specify build
profile, but a test for it is missing. This commit adds a test.
As `mbed-tools compile` supports `-b`, `--profile` for selecting
a build profile, the same should exist for `mbed-tools configure`
as users may want to manually run the build step using `cmake` in
some cases. This commit implements it and supplies a test.

Note: This only affects the build directory - it has no impact on
the contents of `mbed_config.cmake`. To actually build an application
with a profile, a user needs to either run `mbed-tools compile` with
`--profile <profile>`, or manually run `cmake` with
`-DCMAKE_BUILD_TYPE=<profile>` after `mbed-tools configure`.

Fixes ARMmbed#262
@LDong-Arm LDong-Arm force-pushed the configure_profile branch from e1355e2 to 923b263 Compare July 2, 2021 09:19
@Patater Patater merged commit 020c195 into ARMmbed:master Jul 2, 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.

mbed-tools configure generates mbed_config.cmake under develop, and doesn't have a -b option
3 participants