[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Octave-patch-tracker] [patch #9930] [octave forge] (statistics) boxplot
[Octave-patch-tracker] [patch #9930] [octave forge] (statistics) boxplot function with extended features
Sat, 30 May 2020 12:30:24 -0400 (EDT)
Mozilla/5.0 (Windows NT 6.1; rv:52.0) Gecko/20100101 Firefox/52.0
Follow-up Comment #2, patch #9930 (project octave):
First of all thank you for your contribution, Andreas, much appreciated. It's
quite a bit of work, not "just some".
I'm not statistics package maintainer (or maybe I am too,
https://octave.sourceforge.io/statistics/index.html lists "The octave
community" as maintainer and I feel to be part of it).
Yet I have some remarks & questions, mostly (coding) style.
Please don't be put off, Octave is a volunteer project where the devs have
little time and usually don't prioritize spending time on polishing
contributions so we rather have contributors polishing their code themselves:
1. We prefer patches ("diff -u <original_file> <modified_file>"), if possible
even Mercurial csets, so that (A) we have a quick idea of exactly what changed
and (B) the patch or cset can be applied to our local repos directly.
2. Your file uploaded here contained CRLF end-of-lines (Windows style), please
set your editor to LF EOLs.
I hit this when trying "diff -u".
3. In the texinfo header we keep sentences two spaces apart, so two spaces
after a period.
4. We're used to separate if blocks over separate lines and have the entire if
clause in parentheses, so:
if (strcmp (notched, "on"))
notched = 1;
if strcmp (notched, "on") notched = 1; endif
Same for for, while and do-while loops, try-catch and unwind_protect blocks
and whatnot :-)
5. We begin comments with double #, not %, so
## check for string input: 'fixed' or 'proportional' (default if mis-spelled)
% chech for string input: 'fixed' or 'proportional' (default if misspelt)
(I'll grant you the spelling mistakes, I'm no native English speaker either
6. We put each individual statement on a separate line, so:
numarg -= 2;
numarg -= 2; indopt++;
7. We use "! " for negation, not "~", so
if (! isempty
8. You replaced a demo, but IMO you'd better retain it and just add you demo.
If you can upload a patch or cset revised along the above guidelines, I'll
test it and try to compare it with Matlab's output.
Reply to this item at:
Message sent via Savannah