RTEMS | Draft: cpukit: add support for common CAN/CAN FD stack (!49)
Gedare Bloom (@gedare)
gitlab at rtems.org
Thu Jun 27 06:08:20 UTC 2024
Gedare Bloom commented: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/49#note_108389
This is overall very great work, and although I have made many comments, given the amount of code here, it is actually not a high density. Most of my bigger concerns relate to the naming. We want to get this right, as we will most likely be stuck with whatever names we choose at the API level.
There are a few themes I noticed about my comments in here, I'll try to recap:
- API design: The API should more clearly delineate what is internal to the framework, what is provided by a driver, and what the user application should call. The function and struct member names of the APIs are not always consistent, and they use non-standard acronyms. Prefer to spell out names for clarity (e.g., "queue", not "que" or "q"). It is more important to be clear than to be concise.
- Doxygen: we generate doxygen typically from the header files, so prefer to put the Doxygen comments on the function declarations rather than their definitions (except inline functions).
- Code Complexity: Try to avoid complex expressions, when a series of simpler ones suffices. Try to avoid complex code structures and long functions with deep nesting, instead preferring shorter functions and refactoring inner nesting.
And I don't comment on every repetition of the same issue, so make sure to check elsewhere in the code for similar problems as noted in the comments.
--
View it on GitLab: https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/49#note_108389
You're receiving this email because of your account on gitlab.rtems.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/bugs/attachments/20240627/aa08579c/attachment.htm>
More information about the bugs
mailing list