<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.0 Transitional//EN" "http://www.w3.org/TR/REC-html40/loose.dtd">
<html lang="en" style='--code-editor-font: var(--default-mono-font, "GitLab Mono"), JetBrains Mono, Menlo, DejaVu Sans Mono, Liberation Mono, Consolas, Ubuntu Mono, Courier New, andale mono, lucida console, monospace;'>
<head>
<meta content="text/html; charset=US-ASCII" http-equiv="Content-Type">
<title>
GitLab
</title>
<style data-premailer="ignore" type="text/css">
a { color: #1068bf; }
</style>
<style>img {
max-width: 100%; height: auto;
}
body {
font-size: .875rem;
}
body {
-webkit-text-shadow: rgba(255,255,255,.01) 0 0 1px;
}
body {
font-family: var(--default-regular-font, "GitLab Sans"),-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,"Noto Sans",Ubuntu,Cantarell,"Helvetica Neue",sans-serif,"Apple Color Emoji","Segoe UI Emoji","Segoe UI Symbol","Noto Color Emoji"; font-size: inherit;
}
</style>
</head>
<body style='font-size: inherit; -webkit-text-shadow: rgba(255,255,255,.01) 0 0 1px; font-family: var(--default-regular-font, "GitLab Sans"),-apple-system,BlinkMacSystemFont,"Segoe UI",Roboto,"Noto Sans",Ubuntu,Cantarell,"Helvetica Neue",sans-serif,"Apple Color Emoji","Segoe UI Emoji","Segoe UI Symbol","Noto Color Emoji";'>
<div class="content">
<p style="color: #777777;">
<a href="https://gitlab.rtems.org/gedare">Gedare Bloom</a>
<a href="https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/49#note_108389">commented</a>:
</p>
<div class="md" style="color: #333238; word-wrap: break-word;">
<p dir="auto" style="color: #333238; margin: 0 0 16px;" align="initial">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.</p>
<p dir="auto" style="color: #333238; margin: 0 0 16px;" align="initial">There are a few themes I noticed about my comments in here, I'll try to recap:</p>
<ul dir="auto" style="text-align: initial; list-style-type: disc; margin: 0 0 16px; padding: 0;">
<li style="margin-top: 0; line-height: 1.6em; margin-left: 25px; padding-left: 3px;">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.</li>
<li style="line-height: 1.6em; margin-left: 25px; padding-left: 3px;">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).</li>
<li style="line-height: 1.6em; margin-left: 25px; padding-left: 3px;">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.</li>
</ul>
<p dir="auto" style="color: #333238; margin: 0;" align="initial">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.</p>
</div>
</div>
<div class="footer" style="margin-top: 10px;">
<p style="font-size: small; color: #737278;">
—
<br>
<a href="https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/49#note_108389">View it on GitLab</a>.
<br>
You're receiving this email because of your account on <a target="_blank" rel="noopener noreferrer" href="https://gitlab.rtems.org">gitlab.rtems.org</a>. <a href="https://gitlab.rtems.org/-/sent_notifications/7cc30b52bb43574131789e400d7a6332/unsubscribe" target="_blank" rel="noopener noreferrer">Unsubscribe</a> from this thread · <a href="https://gitlab.rtems.org/-/profile/notifications" target="_blank" rel="noopener noreferrer" class="mng-notif-link">Manage all notifications</a> · <a href="https://gitlab.rtems.org/help" target="_blank" rel="noopener noreferrer" class="help-link">Help</a>
<script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","action":{"@type":"ViewAction","name":"View Merge request","url":"https://gitlab.rtems.org/rtems/rtos/rtems/-/merge_requests/49#note_108389"}}</script>
</p>
</div>
</body>
</html>