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

boards/slwstk6000b-*: move CPU_MODEL definition to Makefile.features #12410

Merged

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