Makefile comments and always MODE= #314
Conversation
Already deleted from master
Add many comments. Only functional change: Always include MODE= to allow application_modes to complain about misspelling of mode. PS it appears that git display shows uses different tab stops
|
Can one of the admins verify this patch? |
|
ok to test |
|
The indentation comes out as strange: Let's keep commands in one line, text editors can do word wrap |
src/Makefile
Outdated
| # @file Makefile | ||
| # @author Otso Jousimaa <otso@ojousima.net> | ||
| # @date 2021-03-30 | ||
| # 2021-11-19 DG align options, MODE=APPLICATION_MODE_xxx |
There was a problem hiding this comment.
Version information is in git, please remove duplicate information
There was a problem hiding this comment.
Editor and astyle don't do a good job of wrapping. they sometimes break between keyword and value or just some arbitrary place.
Having all the arguments strung out in one apparent string makes this very confusing (especially with the many underscores). It is difficult to see what is a keyword.
I think that since you are intimately familiar with this and it makes perfect sense to you. I spent a very long time understanding this and was partly making notes so I could remember how it works and hoping that these comments might save others hours of investigation.
Have you looked at it in as a text file where the TABS are better presented?
What's it hurt?
There was a problem hiding this comment.
If there is an effort to format makefiles prettier, it should cover all the makefiles in one go and be based on some standard that everyone can read. For example https://openjdk.java.net/groups/build/doc/code-conventions.html .
While the makefile might look more appealing, there's no good system to catch any possible subtle bugs introduced by this change. For example line
MODE=-DAPPLICATION_MODE_DEFAULT \DEBUG=-DNDEBUG FW_VERSION=-DAPP_FW_VERSION=${VERSION}
has \DEBUG=-DNDEBUG. I have no idea how it evaluates in code with the makefile the string is being passed to. Essentially merging this PR would require testing all the board and firmware variants, and then doing tests again when some other makefile is adjusted for new format.
In general, any formatting changes should be done by a formatting tool that is guaranteed to not change the functionality of the code. If you find such a tool for makefiles and open a pull request for GitHub Action that points out the differences between current file and tool output, I'd be happy to consider using the tool and format it is producing.
There was a problem hiding this comment.
I'll try something different
|
Excellent |
Add many comments.
Only functional change:
Always include MODE= to allow application_modes to complain about misspelling of mode.
PS it appears that git display shows uses different tab stops.