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

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

[Octave-patch-tracker] [patch #7934] Adding tests for operator overloadi


From: Jordi Gutiérrez Hermoso
Subject: [Octave-patch-tracker] [patch #7934] Adding tests for operator overloading in the legacy class system (bug #38128)
Date: Mon, 28 Jan 2013 18:50:28 +0000
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20110109 Sparklemming/3.6.13

Update of patch #7934 (project octave):

                  Status:               Need Info => Done                   
             Open/Closed:                    Open => Closed                 

    _______________________________________________________

Follow-up Comment #3:

> Also, I think that assert(isequal()) is clearer than the
> two-argument syntax, but that's obviously a matter of taste (since
> you call it "ugly").

It's clearer if you're used to Matlab code. Everywhere else in Octave
we use the two- or three-arg assert call when writing tests. After
you've been writing Octave code for a while, Matlab code looks
progressively uglier. ;-)

I also disagree with the comment that you quote above that. Why should
we make this testing on Matlab easier? We don't make this concession
for any other tests in Octave.

Regardless, these are all minor stylistic issues. I may just fix them
later. I have rebased your patch to default, where it's
uncontroversial to add new things, and I've pushed it:

    http://hg.savannah.gnu.org/hgweb/octave/rev/f75ffcc82acb

If we later decide that this should indeed go on stable, we can graft
the patch back into stable.

I have also modified the metadata of your patch. I used your full
name and email as the username of the committer. I also added a commit
message in accordance with these guidelines:

    http://wiki.octave.org/Commit_message_guidelines

I changed the word "legacy". The old-style classes aren't "legacy", at
least not in Octave yet, since we don't have any replacement for them
yet. I furthermore also removed trailing whitespace. If you enable the
colour extension in Mercurial, you will see the trailing whitespace
when you do "hg diff":

    http://mercurial.selenic.com/wiki/ColorExtension

You may wish to remove your local pre-rebased version of the patch by
doing


hg strip -r 56fcf4903a2e4


then pulling the new version from Savannah.

Thanks for the patch.


    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?7934>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

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