Github user mike-jumper commented on a diff in the pull request:
https://github.com/apache/incubator-guacamole-server/pull/74#discussion_r107311953
--- Diff: src/libguac/timestamp.c ---
@@ -33,9 +33,14 @@ guac_timestamp guac_timestamp_current() {
struct timespec current;
- /* Get current time */
+ /* Get current time, monotonically increasing.
+ Using monotonic clock when available, to prevent backward running
time */
--- End diff --
The only other issue I see here with respect to wording is:
> Using monotonic clock when available, to prevent backward running time
seems to simply restate that the time is monotonically increasing, which is
already stated above... essentially, the whole comment reads to me as: "Get
current time, monotonically increasing. Use monotonically increasing time, to
ensure time is monotonically increasing."
If you disagree here and feel it adds something, I'm willing to back off
and let it be, but it should at least be indented to align with the content of
the comment line above it (with leading `*`). I think I may have mentioned this
before in prior review, but if not - my apologies. Currently, it's at the same
level as a normal line of code, which hurts readability.
Searching through the rest of the guacamole-server codebase, it's rare that
we have any comments which span multiple lines, so finding precedent to draw
from is tricky, but there are a few examples of how things look in established
code, such as:
/* Iterate over all the heat map cells for the area
* and calculate the average framerate */
for (y = min_y; y < max_y; y++) {
(see
https://github.com/apache/incubator-guacamole-server/blob/516c4a0593fd78604774bf52c9b547aad5e22619/src/common/surface.c#L395-L396)
Other than that, unless your next revision adds more `printf()` statements,
I think we're good to go. ;)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
