[riot-notifications] [RIOT-OS/RIOT] riotboot: add riotboot_flashwrite module (#11181)
notifications at github.com
Tue Mar 19 14:28:21 CET 2019
I disagree, the fix from https://github.com/RIOT-OS/RIOT/pull/11203 do not fix the issues.
This pull request changed the API without mentioning in the title or giving an adequate testing description.
The third commit https://github.com/RIOT-OS/RIOT/pull/11181/commits/a15f07b04bb3153c69c7dfcb3aa9b5cb23724c07 is added without justification and changes the definition scope of the makefiles variables and made some non namespaced macros globally defined.
This may be correct, but not without a specific discussion and a justification other than "move slot variables". It should have be in its own pull request. Also, if the variables are now parsed by Makefile.include why keep the export ?
If it was required for https://github.com/RIOT-OS/RIOT/pull/11181/commits/ae35860510915461544384ccde57d9455095f18d why is it defined after the commit introducing it ?
And the fix proposed in #11203 now makes that having `FEATURES_REQUIRED+=riotboot` define global non namespaced macros for no reasons.
This should really have been split to a separate pull request with dedicated testing procedure and justification as this pull request did not gave any and did not mention it either.
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the notifications