Apply auto-formatting as implemented in PR #10111#12577
Apply auto-formatting as implemented in PR #10111#12577x3ro wants to merge 1 commit intoRIOT-OS:masterfrom
Conversation
|
It would seem we have to coordinate since these files change so often. I will check through everything now then we should move fast after you rebase. |
MrKevinWeiss
left a comment
There was a problem hiding this comment.
Looks good generally, I would like to replace all BOARD_* += \ with BOARD_* := \, since it only evaluates once it could be faster (though I doubt it would have that big an impact). This means that there can only be one := since additional := overwrites previous values. So maybe just move all comments to the top or bottom then everything will only have one assignment.
Another thing that I would like to run some tests on is moving the include $(RIOTBASE)/Makefile.include to the bottom. Some makefiles will have orders changed, though I don't think it will have any impact it would be nice to just confirm it. I will look into how to achieve it.
|
|
||
| USEPKG += ubasic | ||
| USEMODULE += ubasic_tests | ||
| USEMODULE += printf_float |
| # arduino-mega2560: builds locally but breaks travis (possibly because of | ||
| # differences in the toolchain) | ||
| # The MSP boards don't feature round(), exp(), and log(), which are used in the unittests |
There was a problem hiding this comment.
Maybe we can move all comments to the bottom since everything so far isn't ordered anyways?
| BOARD_BLACKLIST += \ | ||
| chronos \ | ||
| msb-430 \ | ||
| msb-430h \ | ||
| telosb \ | ||
| wsn430-v1_3b \ | ||
| wsn430-v1_4 \ | ||
| z1 \ | ||
| # |
There was a problem hiding this comment.
We can also combine these then to help push the idea of always using := instead of sometimes having +=
| feather-m0 \ | ||
| opencm904 \ | ||
| spark-core \ | ||
| # |
There was a problem hiding this comment.
Maybe move the comment to the top and combine so we can use :=
|
I don't want to drag it on and these things can be attended to in follow-up PRs when it is easier to manage but if it isn't a problem it would be nice to have. |
|
so no need for reformatting |
|
Mmh, I'm honestly not super invested in this change anymore. If it's foreseeable that all of the black- and whitelists will disappear soon then sure, maybe there's no need to merge this. I cannot say I'm able to gauge how long the transition to feature lists will take for all boards. What do you think, @MrKevinWeiss? That being said, should we then also abandon #10111? Or would we want to auto-format |
|
@x3ro, to me #10111 is still valid bc there will be corner-cases where we currently still need some kind of board list. What I wanted to point out here is, that before we reformat existing black/white lists we should look which of these can easily be replaced by using the feature required or blacklist method. Which means this PR is not completely invalid but not all changes are necessary or at least we can/should avoid changing them twice in short succession. |
I think I disagree with this, i.e. I don't see much of a problem with changing them in short succession, and there's always the risk that a supposedly easy change (e.g. removing the blacklists) takes longer than expected, thus in this case also delaying this here pull request. That being said, I don't feel strongly about it and thus will leave this open until the relevant changes on the black- and whitelist side are done, so that I can update. Please ping me here in this issue once this is done @smlng or @aabadie or whoever is spearheading that task :) |
|
I would like this done sooner than later. It should be pretty simple clean everything at least. Additional cleanups should benefit from working with cleaner makefiles. Alternatively, we can just push the formatting script as a tool and not with the CI and let whoever wants to get the makefiles up to a standard with all the features grouped take care of it. Then once everything fits the script invoke it with murdock. |
|
ping |
|
I think we need consensus here which way to go first, either this PR or postpone until board blacklists have been replaced by proper feature specs. |
|
What is the state on the proper feature specs? |
They are in. Care to rebase / rework this? |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions. |
Let's get PR auto-formatting merged! 🎉 This replaces #10130 in the hopes of having a short discussion/review and then being able to merge!
Once this is merged I'd then push forward and get #10111 wrapped up, followed by a PR that integrates it into the build system.
🚂