Skip to content

tests: Moved BOARD_INSUFFICIENT_MEMORY to Makefile.ci#12484

Merged
benpicco merged 3 commits intoRIOT-OS:masterfrom
maribu:pain_in_the_ass
Oct 17, 2019
Merged

tests: Moved BOARD_INSUFFICIENT_MEMORY to Makefile.ci#12484
benpicco merged 3 commits intoRIOT-OS:masterfrom
maribu:pain_in_the_ass

Conversation

@maribu
Copy link
Member

@maribu maribu commented Oct 17, 2019

Contribution description

  • Moved BOARD_INSUFFICIENT_MEMORY from Makefile to Makefile.ci
  • Formatted the list to have one board per line
  • Sorted the lines alphabetically
  • Adapted the CI tests to enforce that BOARD_INSUFFICIENT_MEMORY does not sneak back into Makefiles

Testing procedure

  • Murdock will do the hard ware here, no C code has been changed

Issues/PRs references

Follow up to #12406

@maribu maribu requested a review from smlng October 17, 2019 09:36
@maribu maribu added 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 labels Oct 17, 2019
smlng
smlng suggested changes Oct 17, 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.

minor comment(s), otherwise looks good. If Murdock is happy this should be good to go.

@smlng smlng added the Area: build system Area: Build system label Oct 17, 2019
@smlng smlng self-assigned this Oct 17, 2019
@maribu
Copy link
Member Author

maribu commented Oct 17, 2019

@smlng: Thanks for nothing that. I added the missing \\\n # and squashed right away.

@benpicco
Copy link
Contributor

benpicco commented Oct 17, 2019

Damn, I should have hit the green button on #12411 already 😉 (or just waited a couple more days)

benpicco
benpicco previously approved these changes Oct 17, 2019
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Same change as for examples, makes maintaining those lists much easier.

smlng
smlng previously approved these changes Oct 17, 2019
View reviewed changes
@smlng
Copy link
Member

smlng commented Oct 17, 2019

CI not happy, some whitespace error, please fix and amend directly.

@benpicco benpicco mentioned this pull request Oct 17, 2019
Merged
@maribu
Copy link
Member Author

maribu commented Oct 17, 2019

Please wait a moment before merging

@maribu maribu force-pushed the pain_in_the_ass branch 2 times, most recently from cad540b to 597900d Compare October 17, 2019 12:04
@maribu
Copy link
Member Author

maribu commented Oct 17, 2019

Using the silver searcher I could use this to find some Makefile.ci that were differently typeset:

ag -lv -G 'Makefile\.ci' '(#[^\n]*\n)*BOARD_INSUFFICIENT_MEMORY := \\\n(    [a-zA-Z0-9-_]+ \\\n)+    #\n'

Maybe some check like this could be added to travis, if someone knows how to do this best?

@maribu maribu dismissed stale reviews from smlng and benpicco October 17, 2019 12:41

Code has changed since. (But I would happy if you could ACK again!)

@benpicco
Copy link
Contributor

benpicco commented Oct 17, 2019

I think the computer can do a much better job at keeping those lists properly formatted, humans shouldn't be required to meticulously craft those manually.

@benpicco
Copy link
Contributor

benpicco commented Oct 17, 2019

ACKs don't loose their validity if you push something 😉

@maribu
Copy link
Member Author

maribu commented Oct 17, 2019 •
edited
Loading

ACKs don't loose their validity if you push something 😉

I think that @MichelRottleuthner told me during FGSN that he wouldn't like it if I would wait for his ACK, than force push completely different stuff and hit the merge button before he can revoke his ACK, and then blame him cause he gave the ACK. (I cannot recall why he told me that so specifically 😆)

@maribu
Copy link
Member Author

maribu commented Oct 17, 2019

Oha. Merge conflict 😢 I'm on it.

maribu added 2 commits October 17, 2019 15:10
@maribu
tests/rng: Fixed abuse of BOARD_BLACKLIST
d9e4361 
- BOARD_BLACKLIST has been used to blacklist boards with too little RAM/ROM
  according to the comment
  ==> Moved those entries to BOARD_INSUFFICIENT_MEMORY instead
- pic32-clicker does build fine, so RAM/ROM efficiency has improved since
  ==> Dropped pic32-clicker from the list
@maribu
tests: BOARD_INSUFFICIENT_MEMORY -> Makefile.ci
fd6feda 
- Moved the BOARD_INSUFFICIENT_MEMORY list into Makefile.ci
- Formatted the list to contain one board per line
- Sorted the lists alphabetically
@benpicco
Copy link
Contributor

benpicco commented Oct 17, 2019

Just squash the last two commits.

@maribu
dist/tools: Check BOARD_INSUFFICIENT_MEMORY usage
b6d23b5 
- Enforce that all applications do not manage BOARD_INSUFFICIENT_MEMORY in the
  Makefile
- Match also "BOARD_INSUFFICIENT_MEMORY +=", not only
  "BOARD_INSUFFICIENT_MEMORY :=" or "BOARD_INSUFFICIENT_MEMORY =".
benpicco
benpicco approved these changes Oct 17, 2019
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

The code is still good.

@benpicco benpicco merged commit ad81a9d into RIOT-OS:master Oct 17, 2019
This was referenced Oct 17, 2019
Merged
Merged
@maribu maribu deleted the pain_in_the_ass branch November 4, 2019 19:54
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

+1 more reviewer

@smlng smlng smlng left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@smlng smlng

Labels

Area: build system Area: Build system 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 2020.01

Development

Successfully merging this pull request may close these issues.

4 participants

@maribu @benpicco @smlng @fjmolinas