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.

Sign up for GitHub

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

Jump to bottom

boards/cpu: cleanup duplicate includes #9923

Merged
merged 2 commits into from
Sep 19, 2018
Merged

boards/cpu: cleanup duplicate includes #9923

merged 2 commits into from
Sep 19, 2018

Conversation

cladmi
Copy link
Contributor

@cladmi cladmi commented Sep 11, 2018 •
edited
Loading

Contribution description

Remove duplicate includes of $(CPU)/include and $(BOARD)/include.

Includes of $(CPU)/include and $(BOARD)/include and already added in the
main Makefile.include.

Testing procedure

I tested that no header conflict in cpu and boards:

find cpu -name \*.h -exec basename {} \; | sort -u > cpu_includes
find boards/ -name \*.h -exec basename {} \; | sort -u > boards_includes

There is only one filename in common:

cat cpu_includes boards_includes  | sort -u | wc -l
907
cat cpu_includes boards_includes  | wc -l
908

find cpu/ -name periph_conf.h 
cpu/native/include/periph_conf.h

Issues/PRs references

Part of #8713
Found when working on #9913

@cladmi
boards/Makefile.include: remove duplicate includes
6fc8fae 
Includes of $(CPU)/include and $(BOARD)/include and already added in the
main Makefile.include.

Verification:

There are no common headers names between boards and cpus.
Except native that has a 'periph_conf.h' in cpu instead of being in board.
@cladmi
cpu/cc430: remove duplicate include
28d3ba6 
Includes of $(CPU)/include is already included in the main Makefile.include
in the line before including this file.
@cladmi cladmi added Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports labels Sep 11, 2018
@leandrolanzieri
Copy link
Contributor

I just ran the test that is failing in Murdock and works fine. Also, it doesn't seem to be related to this changes. Maybe re-trigger it?

@cladmi
Copy link
Contributor Author

cladmi commented Sep 12, 2018

New unreliable test youhou ! Let's re-run tests.

@cladmi cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 12, 2018
@cladmi cladmi added this to the Release 2018.10 milestone Sep 19, 2018
@cladmi cladmi requested a review from aabadie September 19, 2018 11:45
aabadie
aabadie approved these changes Sep 19, 2018
View reviewed changes
Copy link
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Looks good, ACK

@aabadie aabadie merged commit 1022050 into RIOT-OS:master Sep 19, 2018
@cladmi cladmi deleted the pr/cleanup/duplicate_includes branch September 19, 2018 15:27
@cladmi
Copy link
Contributor Author

cladmi commented Sep 20, 2018

Thanks for reviewing.

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

@aabadie aabadie aabadie approved these changes

Assignees
No one assigned
Labels
Area: boards Area: Board ports Area: build system Area: Build system Area: cpu Area: CPU/MCU 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
Projects
None yet
Milestone
Release 2018.10
Development

Successfully merging this pull request may close these issues.
3 participants
@cladmi @leandrolanzieri @aabadie