-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rfc7: amend C style recommendations #435
Conversation
Problem: the C section has subsections but the original whitespace oriented content is not in a subsection. Create a "whitespace" subsection.
Problem: we've adopted a defacto guideline that long parameter lists should be broken to one per line but it is not documented. Add this to RFC 7.
Problem: we have internalized an expectation that functions do not have side effects on error, but this is undocumented. Add this to RFC 7.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
9. Long parameter lists in function declarations or calls SHOULD be broken | ||
to one parameter per line, at the same indent level. Example: | ||
|
||
.. code-block:: c | ||
|
||
flux_future_t *flux_rpc (flux_t *h, | ||
const char *topic, | ||
const char *s, | ||
uint32_t nodeid, | ||
int flags); | ||
|
||
.. code-block:: c | ||
|
||
f = flux_rpc (h, | ||
"service.do-something", | ||
NULL, | ||
FLUX_NODEID_ANY, | ||
FLUX_RPC_RESPONSE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
little late to the party no this, but do we also want to add a similar thing for complex branch statements? i.e
if (foobar == 1
|| boobar > something
|| cow < 123434343)
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we should!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hehe, ok i'll write up an attempt
Problem: we have organically adopted some project norms for C code that are not documented.
Update RFC 7.