Skip to content

cpu/samX: remove guards from periph drivers#7984

Merged
haukepetersen merged 3 commits intoRIOT-OS:masterfrom
haukepetersen:opt_sam_periphfileguards
Nov 9, 2017
Merged

cpu/samX: remove guards from periph drivers#7984
haukepetersen merged 3 commits intoRIOT-OS:masterfrom
haukepetersen:opt_sam_periphfileguards

Conversation

@haukepetersen
Copy link
Copy Markdown
Contributor

@haukepetersen haukepetersen commented Nov 9, 2017 •
edited
Loading

addresses #7981
same as #7982

@haukepetersen haukepetersen added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 9, 2017
@haukepetersen haukepetersen mentioned this pull request Nov 9, 2017
Closed
46 tasks
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Nov 9, 2017

fixed #7981 (partly)

The problem when writing this is that #7981 will be closed when this PR will be merged...

@dylad
Copy link
Copy Markdown
Member

dylad commented Nov 9, 2017

Changes look good. I'll give it a try later today.
Murdock complains about sam0 i2c but this should be fix with #7588

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Nov 9, 2017

@aabadie 'addresses' won't lead to automatic closing, right?

@dylad: thanks for reviewing. Will fix the cppcheck issues for now (temporarily) and #7588 can than take care of it for good...

@haukepetersen haukepetersen force-pushed the opt_sam_periphfileguards branch from 10fdc20 to ec27204 Compare November 9, 2017 12:53
@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented Nov 9, 2017

'addresses' won't lead to automatic closing, right?

addresses seems to be fine. I'm wondering if Github will take the change into account since you initially used fixes

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Nov 9, 2017

we'll see.

@dylad
Copy link
Copy Markdown
Member

dylad commented Nov 9, 2017

I tested this PR, I introduces some errors within driver & try to compile them. Errors are only show if I add the required USEMODULE += periph_xxx. It works like a charm.
There is no need to test it on real hardware right ?

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Nov 9, 2017

There is no need to test it on real hardware right ?

I would say no. If I did anything wrong with this PR, it would IMHO only lead to failed builds (by building more stuff than necessary). So if everything still builds correctly, all should be in order.

dylad
dylad approved these changes Nov 9, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

Re-run some tests on differents boards & also check build size. Everything looks good. -> ACK

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented Nov 9, 2017

thanks a lot!

All green -> go

@haukepetersen haukepetersen merged commit 5c31d20 into RIOT-OS:master Nov 9, 2017
@haukepetersen haukepetersen deleted the opt_sam_periphfileguards branch November 9, 2017 16:26
@aabadie aabadie added this to the Release 2018.01 milestone Jan 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dylad dylad dylad approved these changes

Assignees

@aabadie aabadie

@dylad dylad

Labels

Area: drivers Area: Device drivers 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.01

Development

Successfully merging this pull request may close these issues.

3 participants

@haukepetersen @aabadie @dylad