Skip to content

boards/common: use Makefile.features#11675

Merged
MrKevinWeiss merged 4 commits intoRIOT-OS:masterfrom
cladmi:pr/boards/common/use_makefile_features
Jul 2, 2019
Merged

boards/common: use Makefile.features#11675
MrKevinWeiss merged 4 commits intoRIOT-OS:masterfrom
cladmi:pr/boards/common/use_makefile_features

Conversation

@cladmi
Copy link
Contributor

@cladmi cladmi commented Jun 11, 2019 •
edited by MrKevinWeiss
Loading

Contribution description

Update boards to use boards/common/*/Makefile.features if they were using boards/common/*/Makefile.include already.
This de-duplicates the including on $(RIOTCPU)/cpu_name/Makefile.features.

Also now the include $(RIOTCPU)/cpu_name/Makefile.features is done in the same module that defines CPU.

It is a cleanup to simplify moving CPU/CPU_MODEL to Makefile.features.

Testing procedure

info-build gives the same output as in master for all boards with only the newly created file in MAKEFILE_LIST.

BOARDS=$(git diff --name-status 9b4c01c2dd5cdba222804f222acc79ee1ba1cda9 | grep -v 'common' | sed 's/M\t*//;s/^[^/]*\///;s/\/Makefile.features//')
echo ${BOARDS}
# avsextrem blackpill bluepill firefly msb-430 msb-430h msba2 remote-pa remote-reva remote-revb
for board in ${BOARDS}; do BOARD=${board} make --no-print-directory -C examples/hello-world/ info-build; done 

diff  output_master output_pr
131a132
>       /home/harter/work/git/RIOT/boards/common/msba2/Makefile.features  
690a692
>       /home/harter/work/git/RIOT/boards/common/remote/Makefile.features  
845a848
>       /home/harter/work/git/RIOT/boards/common/msb-430/Makefile.features  
996a1000
>       /home/harter/work/git/RIOT/boards/common/msb-430/Makefile.features  
1159a1164
>       /home/harter/work/git/RIOT/boards/common/msba2/Makefile.features  
1336a1342
>       /home/harter/work/git/RIOT/boards/common/remote/Makefile.features  
1514a1521
>       /home/harter/work/git/RIOT/boards/common/remote/Makefile.features  
1692a1700
>       /home/harter/work/git/RIOT/boards/common/remote/Makefile.features

Issues/PRs references

Cleanup part of: Tracking: move CPU/CPU_MODEL to Makefile.features #11477
Related to #11674

@cladmi cladmi added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: boards Area: Board ports labels Jun 11, 2019
@cladmi cladmi added this to the Release 2019.07 milestone Jun 11, 2019
@cladmi cladmi force-pushed the pr/boards/common/use_makefile_features branch from 7ed521a to f5bef68 Compare June 11, 2019 15:17
@cladmi
Copy link
Contributor Author

cladmi commented Jun 11, 2019

Updated the commit messages as they were not explaining why the change was done.

@cladmi cladmi requested a review from MrKevinWeiss June 27, 2019 14:03
@cladmi cladmi added the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Jun 28, 2019
@cladmi cladmi force-pushed the pr/boards/common/use_makefile_features branch from f5bef68 to 7466f18 Compare June 28, 2019 10:00
@cladmi
Copy link
Contributor Author

cladmi commented Jun 28, 2019

Rebased now that FEATURES_MCU_GROUP was removed. I updated the test procedure to match the reference commit hash and verified the output.

cladmi added 4 commits July 1, 2019 18:09
@cladmi
boards/bluepill/blackpill: always include common Makefile.features
38e0ce3 
The file is there so no reason to have an include if file exists.
@cladmi
boards/msp30: use common/msb-430/Makefile.features
6be662a 
Use the common `msb-430/Makefile.features` instead of duplicating the
`cpu` include.
@cladmi
boards/msba2-based: use common/msba2/Makefile.features
5b00d3a 
Introduce and use a common/msba2/Makefile.features.
The boards were already using the common `Makefile.include` and
`Makefile.dep`.

This makes 'include $(RIOTCPU)/lpc2387/Makefile.features' be done in
the same module where 'CPU' is defined.

Preparation for moving 'CPU' definition to Makefile.features.
@cladmi
boards/remote: use common/remote/Makefile.features
ec31249 
Introduce and use a common/remote/Makefile.features.
The boards were already using the common `Makefile.include` and
`Makefile.dep`.

This makes 'include $(RIOTCPU)/cc2538/Makefile.features' be done in
the same module where 'CPU' is defined.

Preparation for moving 'CPU' definition to Makefile.features.
@cladmi cladmi force-pushed the pr/boards/common/use_makefile_features branch from 7466f18 to ec31249 Compare July 1, 2019 16:10
@cladmi cladmi mentioned this pull request Jul 1, 2019
Closed
18 tasks
@cladmi cladmi changed the title boards/common: use Makeflie.features boards/common: use Makefile.features Jul 1, 2019
smlng
smlng approved these changes Jul 2, 2019
View reviewed changes
Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

Looks sound, ACK

@smlng smlng added 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: 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 Jul 2, 2019
@MrKevinWeiss MrKevinWeiss added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jul 2, 2019
@MrKevinWeiss MrKevinWeiss merged commit 2ebd65f into RIOT-OS:master Jul 2, 2019
@cladmi cladmi deleted the pr/boards/common/use_makefile_features branch July 4, 2019 09:58
@cladmi
Copy link
Contributor Author

cladmi commented Jul 4, 2019

Thanks for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

1 more reviewer

@smlng smlng smlng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer 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

Milestone

Release 2019.07

Development

Successfully merging this pull request may close these issues.

3 participants

@cladmi @smlng @MrKevinWeiss