On Freitag, 26. August 2016 15:02:51 CEST Jonathan Haws wrote: [...]
alfred: Externalized synchronization interval
ALFRED_INTERVAL is now externalized via the -p option (synchronization period). If specified as option, user supplied interval is used, otherwise the default interval of ALFRED_INTERVAL is used.
Does that look right?
Now the "Signed-off-by: Jonathan Haws Jonathan.Haws@sdl.usu.edu" is missing at the end. You should check out "11) Sign your work" [2] to understand why Linux related submissions tend to have this line.
The reason why it must be at the end was shown in my last mail. Tools like git-am or git-format-patch can automatically add the Signed-off-by of the person who accepts/forwards the patch - but only does it at the end of the commit message. So you Signed-off-by should also be at the end of the commit message.
index c5df945..884a1a7 100644 --- a/server.c +++ b/server.c @@ -372,7 +372,10 @@ int alfred_server(struct globals *globals)
while (1) { clock_gettime(CLOCK_MONOTONIC, &now);
time_diff(&now, &globals->sync_period, &now);
/* subtract the synchronization period from thecurrent time */
time_subtract(&now, &globals->sync_period, &now);I'm guessing Simon meant to have the comment in the first patch. I don't get why you now introduce a time_subtract which is just a copy of time_diff (without the return value).
My reading of his response was that time_diff was meant for something completely different, thus shouldn't be used here. Thus I created time_subtract.
My reading was that Simon wanted just a comment which explains what is does (because it was not intuitive to him).
@Simon: can you clarify it for both of us? See also the following question from Jonathan:
[...]
Yeah, I've looked at that. The code they present there essentially does the same thing. I can pull that in and have a timespec_subtract call, or I can go back to time_diff with the comment. Which would you prefer?
(see above)
- return;
+}
Why a return here?
Style. I never have a function without a return, even voids. Maybe I'm a bit OCD on that point, but I just feel happier seeing it there. :) Depending on what you want to do with time_subtract, we can remove this return or replace the function.
We prefer not to have a wasted return [4]. But lets wait for Simon's answer regarding the extra function.
Btw. I am seeing whitespace warnings when applying the patches. Maybe you can check them:
Applying patch #16667 using 'git am' Description: [1/2] alfred: Externalized synchronization interval Applying: alfred: Externalized synchronization interval .git/rebase-apply/patch:89: trailing whitespace. ALFRED_INTERVAL setting of 10 seconds will be used. Fractional seconds are warning: 1 line adds whitespace errors. Applying patch #16668 using 'git am' Description: [2/2] alfred: Adding time_subtract utility function Applying: alfred: Adding time_subtract utility function
I'm not sure where this would have cropped up, but I'll double check it before I submit the next patch version.
Looks like following line in the manpage has an extra space at the end:
+ALFRED_INTERVAL setting of 10 seconds will be used. Fractional seconds are
I'm new to the contributing game - I've read through the wiki, but apparently there are other details that I missed. Is there some good documentation on the typical procedure to follow and formats and whatnot to read up on? I thought I had understood everything, but apparently not. I hate to cause extra work for you maintainers simply because I can't get a simple patch right.
batman-adv is a a kernel module (included in the official Linux kernel) and thus we have to follow the linux kernel coding style [1] and submission guidelines [2]. The batctl and alfred patches use the same style and we want to have patches have submitted the same way [3]. But we don't tend to enforce everything for alfred/batctl because these patches don't have to be forwarded to the next "higher" maintainer. But odd things will still be pointed out. And they will also sometimes result in rejections but this depends on the maintainer and level of "oddity",
Kind regards, Sven
[1] https://www.kernel.org/doc/Documentation/CodingStyle [2] https://www.kernel.org/doc/Documentation/SubmittingPatches [3] https://www.open-mesh.org/projects/open-mesh/wiki/Contribute#Submitting-patc... [4] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/scripts... search for RETURN_VOID