Skip to content

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Oct 9, 2019

Contribution description

This is still currently a hack to hardcode it as the value can be deduced
from the BOARD_MODULE daughter board name.

But it requires more cleanup and could come in a separate step.

Part of moving CPU/CPU_MODEL definition to Makefile.features to have it
available before Makefile.include.

There is now no more boards defining CPU_MODEL in Makefile.include.

Testing procedure

The value of CPU_MODEL and the output of info-build stayed the same:

for board in slwstk6000b-slwrb4150a slwstk6000b-slwrb4162a; do BOARD=${board} make --no-print-directory -C examples/hello-world/ info-debug-variable-CPU_MODEL info-build; done

The dist/tools/buildsystem_sanity_check/check.sh executes without error (done by CI).

Issues/PRs references

This is still currently a hack to hardcode it as the value can be deduced
from the `BOARD_MODULE` daughter board name.

But it requires more cleanup and could come in a separate step.

Part of moving CPU/CPU_MODEL definition to Makefile.features to have it
available before Makefile.include.
@fjmolinas fjmolinas added Area: boards Area: Board ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: build system Area: Build system labels Oct 9, 2019
@fjmolinas fjmolinas self-requested a review October 9, 2019 16:57
@fjmolinas fjmolinas added this to the Release 2019.10 milestone Oct 9, 2019
@fjmolinas
Copy link
Contributor

I agree with the changes in this PR, although a temporary solution it allows to finish the CPU_MODEL, CPU migration to Makefile.features. This can be cleaned up once an agreement is reached on how to proceed as a result of #12407.

@fjmolinas
Copy link
Contributor

@kb2ma this would allow closing #11477. You changed the mileston for that issue, are you OK with getting this and allow to finish #11477?

@fjmolinas
Copy link
Contributor

Tested that there is no diff by running:

for board in slwstk6000b-slwrb4150a slwstk6000b-slwrb4162a; do BOARD=${board} make --no-print-directory -C examples/hello-world/ info-debug-variable-CPU_MODEL info-build; done

in master an this PR.

@fjmolinas fjmolinas added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Oct 9, 2019
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!

I won't merge unless you agree @kb2ma agrees since you changed the milestone, but IMO this is safe to merge once murdock is green.!

@kb2ma
Copy link
Member

kb2ma commented Oct 9, 2019

Sure, if you two are comfortable with this plan, so am I. I'll reset the milestone on #11477. If something pops up, we can re-evaluate tomorrow, 10 Oct.

@fjmolinas
Copy link
Contributor

Sure, if you two are comfortable with this plan, so am I. I'll reset the milestone on #11477. If something pops up, we can re-evaluate tomorrow, 10 Oct.

Then GO!

@fjmolinas fjmolinas merged commit e2eaf3e into RIOT-OS:master Oct 9, 2019
@cladmi
Copy link
Contributor Author

cladmi commented Oct 10, 2019

Thank you for the review.

@cladmi cladmi deleted the pr/boards/slwstk6000b/cpu_model_hack branch October 10, 2019 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: boards Area: Board ports Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants