[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Octave-patch-tracker] [patch #8912] add function reducepatch
From: |
Lachlan Andrew |
Subject: |
[Octave-patch-tracker] [patch #8912] add function reducepatch |
Date: |
Fri, 19 Feb 2016 22:37:13 +0000 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:43.0) Gecko/20100101 Firefox/43.0 |
Follow-up Comment #1, patch #8912 (project octave):
Thanks, Markus.
I have a few trivial comments on coding style:
- Please avoid long lines. I think the recommended limit is
about 72 or 76 chars, although I normally use up to 80.
Strings can be broken by ["splitting them ", "into arrays ",
"like this"], and breaking at commas.
- For the for loop, please place the "var =" inside
the parentheses.
- Please place spaces around "+"; they're missing in a few
"...+1" expressions.
In the doc string, it would be nice for the first sentence to be complete
(including "the" and full stop), and perhaps be more informative. Perhaps:
"Reduce the number of patch faces and vertices in a <what type of object? the
output of a particular command/package?>, while retaining the patch outline."
Also, "+= 1" is currently twice as fast as "++". You may want to avoid ++.
(I can't guarantee that these changes will help the patch to be applied by
someone authorised to do that, but I hope they may.)
_______________________________________________________
Reply to this item at:
<http://savannah.gnu.org/patch/?8912>
_______________________________________________
Message sent via/by Savannah
http://savannah.gnu.org/