Skip to content

examples, tests: Clean BOARD_* list in Makefiles#9983

Closed
MrKevinWeiss wants to merge 8 commits intoRIOT-OS:masterfrom
MrKevinWeiss:pr/cleanup/make/boards
Closed

examples, tests: Clean BOARD_* list in Makefiles#9983
MrKevinWeiss wants to merge 8 commits intoRIOT-OS:masterfrom
MrKevinWeiss:pr/cleanup/make/boards

Conversation

@MrKevinWeiss
Copy link
Copy Markdown
Contributor

Contribution description

This PR rearranges the BOARD_INSUFFICIENT_MEMORY, BOARD_BLACKLIST and BOARD_WHITELIST to make add in new boards easier.

Testing procedure

Passes murdock and review.

Issues/PRs references

#9965

@MrKevinWeiss MrKevinWeiss added State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet Area: build system Area: Build system Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Sep 20, 2018
@MrKevinWeiss MrKevinWeiss self-assigned this Sep 20, 2018
@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

I still need to run through the blacklist and whitelist (tomorrow morning).

Should we go for this until a better solution is out or should I just leave it in favor of #9081?

@kaspar030
Copy link
Copy Markdown
Contributor

@MrKevinWeiss thanks for tackling this! Honestly, I prefer the shorter representation (as it is now, as a short but wide block). How did you change the files? Maybe we can come up with some script?

@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

Ya I used a script but just quick and dirty python one (and looks pretty bad so I didn't want it merged). If you say no that is fine but every time I or someone adds a board they should push the rest of the boards down. Not the end of the world but seems unnecessary.

Your call @kaspar030, I just wanted to follow through with what I said I will do when ACKing the blackpill board.

@MrKevinWeiss MrKevinWeiss added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet labels Sep 21, 2018
@kaspar030
Copy link
Copy Markdown
Contributor

Any other opinions on line count vs. editability here?

@MrKevinWeiss how much work would it be to change your script to output blocks with line length < 80?

@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

@kaspar030 I would probably just rewrite but my goal is to have one line per board making future maintenance easier. Like I said if you feel strongly against it I am not going to push for this and we can close.

@miri64 miri64 added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Sep 21, 2018
@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

@cladmi I would either like to (fix and) merge to close this PR. Can you throw in your two cents so we can make a decision?

@x3ro
Copy link
Copy Markdown
Contributor

x3ro commented Oct 5, 2018

I'm just quoting my reason for preferring the "one board per line" option, hoping that it might get this merged :)

@MrKevinWeiss @kaspar030 In regards to the "one board per line" vs. "80 characters per line" question, I favor one board per line because it results in much cleaner diffs. For example, in the 80 characters per line case, if I wanted to change the first board in a list, this would likely change all following lines of the list in the diff.

@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented Oct 6, 2018

I re-arranged many applications that were not building for some boards and it was a pain.
Simplifying these task for dev would be better for me.

And one board per line does indeed help with this.

But as there is a tool that may do this, I would say it could be better to merge it along with enabling the tool check in CI.

@MrKevinWeiss
Copy link
Copy Markdown
Contributor Author

Alright closing in favor of #10111

@MrKevinWeiss MrKevinWeiss deleted the pr/cleanup/make/boards branch October 18, 2018 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: build system Area: Build system CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable 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

Development

Successfully merging this pull request may close these issues.

5 participants