octave-patch-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-patch-tracker] [patch #9930] [octave forge] (statistics) boxplot


From: Philip Nienhuis
Subject: [Octave-patch-tracker] [patch #9930] [octave forge] (statistics) boxplot function with extended features
Date: Sat, 30 May 2020 12:30:24 -0400 (EDT)
User-agent: 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;
endif

rather than

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)

rather than

% 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;
indopt++;

rather than

numarg -= 2; indopt++;


7. We use "! " for negation, not "~", so

if (! isempty 

instead of

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:

  <https://savannah.gnu.org/patch/?9930>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]