octave-maintainers
[Top][All Lists]
Advanced

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

submitting changesets (was imwrite)


From: Thomas L. Scofield
Subject: submitting changesets (was imwrite)
Date: Mon, 25 Aug 2008 10:53:06 -0400


On Aug 20, 2008, at 5:35 PM, John W. Eaton wrote:
On 17-Aug-2008, Thomas L. Scofield wrote:

| Here is a large changeset, mostly addressing functionality in  
| imwrite.  I've made a change to imread (really __magick_read__ and/or  
| the functions it calls) so that frame data is always returned in the  
| 4th coordinate.  Imwrite now should handle frames and color maps.  I  
| have a start at handling options---right now only  'Quality' setting  
| is handled when writing a .jpg file.

This patch appears to have been mangled by a mailer.  To avoid
problems like this, please send changesets as attachments (preferably
with the content-type set to "text/plain").

Also, please don't untabify the files.  Doing that makes the diff
unnecessarily larger as there are things like

| - error ("imwrite: colormap must not be empty");
| +        error ("imwrite: colormap must not be empty");

which are not really differences but make it harder to evaluate the
patch.

| +      isParamList = 1;

Please don't use CamelCase variables.  Write is_param_list instead.

| +#include <math.h>

Please don't include C language headers in C++ code.  Use <cmath>
instead.

| +#include <ov-struct.h>

Please use

  #include "ov-struct.h"

for Octave headers in Octave code.  Also, please keep the list of
headers alphabetized.

Will you please make these changes and resend the changeset as an
attachment?

What do Mac users who use Mail.App do to submit changesets?  In the past I have attached a text file to my message only to hear it came through as some sort of binary file.  This last time I included the text in my email message, only to hear the changeset was mangled somehow.  I'm just not willing to employ a different mailer just for Octave submissions.

I don't understand why CamelCase variables are undesirable---a good variable name can be several characters shorter that way.  Nevertheless, when I review the contrib.txi, I do see the instruction there to avoid mixed-case variable names.  Some other conventions mentioned here are new to me (where documented?).  Octave header files are still something of a mystery to me---why is it oct.h is out in lieu of config.h for functions included in Octave core?

As for the tabbing, I cannot both follow the request of the administrator and the coding rules in contrib.txi.  The "have no tabs in your code" rule is one that actually makes good sense to me (unlike some of the other rules expressed in contrib.txi that I follow).  The original files in question were not easily readable in my editor until I turned the previous author's tabs into spaces.  This must be why the rule was established in the first place---sharing code with someone else whose editor renders tabs with a different number of spaces than your editor is annoying.  It is regrettable that this makes more work for the administrator, but rules are rules and, in this case, I'm the one bringing the code INTO compliance.  I cannot easily now return the tabs to their original state, anyway.

In making the other changes, the easiest thing would be to re-send my original changeset, make the modifications, and then send diffs on the original diff.  That, I'm sure, is not what the administrator wants (I surely wouldn't!).  What is the right way to do this?  Though it is not overly difficult to do, surely there is something more direct than downloading the whole repository over again and once again replacing the files I've changed with my versions of them.  It is not clear to me how to use hg revert (perhaps it doesn't work after you commit a change?), and hg rollback looks like it would be an easy command to misuse.

Thanks,

jwe

Thomas L. Scofield
--------------------------------------------------------
Associate Professor
Department of Mathematics and Statistics
Calvin College
--------------------------------------------------------


reply via email to

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