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

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

[Octave-patch-tracker] [patch #9014] Add "camlight" function


From: Markus Mützel
Subject: [Octave-patch-tracker] [patch #9014] Add "camlight" function
Date: Mon, 11 Jul 2016 19:26:40 +0000 (UTC)
User-agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:48.0) Gecko/20100101 Firefox/48.0

Follow-up Comment #10, patch #9014 (project octave):

Colin, I did not have time to test your function thoroughly. Maybe this should
be done by someone that is more experienced and can push changes anyway.
I found only some very minor issues while reading the code and running the
demos and tests (which btw. all pass for me on Ubuntu 16.04 and Windows 10).

During the tests you add up to 10 lights to the same axes. That might trigger
unrelated OpenGL warnings (or other noise) on some systems. You might consider
using "unwind_protect" constructs with "unwind_protect_cleanup" blocks to
close the hidden figure you use in your tests. However, I do not know whether
that can (or should) span several tests.
Or you could delete the lights when you no longer need them in your tests?

Errors should start with the function name + ": ".

You should add a few more words to the error messages (or the complete one) in
the tests to make it clearer for which error you are testing in which test.

You should add "__rotate_around_axis__.m" to scripts/plot/util/module.mk.

You could add a test for the error "if first arg is a handle, it must be a
light handle". 

And finally a little bit of nitpicking: Comments that start at the beginning
of a line should start with ##. I think this also holds for comments in demos
and tests.

I am looking forward to seeing this in 4.2.0.

    _______________________________________________________

Reply to this item at:

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

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




reply via email to

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