[Octave-patch-tracker] [patch #9067] java sources part 1

[Mike Miller]

Follow-up Comment #6, patch #9067 (project octave):

I took 5 minutes and reviewed very briefly. I can offer some basic code
formatting suggestions.

* Please delete all trailing whitespace at the end of every line.
* Watch your indentation, I see at least one place that is indented by an
extra space, around line 330 of ClassHelper.java.
* Do not break function arguments on every comma, only break when the line
exceeds 80 columns.
* I don't know what these "PMD" and "NOPMD" comments mean.
* Don't comment out code that should just be deleted, that's what hg is for.
* Is it really so bad to keep "else if" after a case that does a return? The
"else" at the start of the line makes it clear that the cases are related
alternatives, otherwise you have to look into the if block to see whether it
returns something or not.

If you want to make changes to your patch, please fold those changes into and
refresh the existing patch.

I have no comments on the rest, it's Java that is over my head and too deep to
review quickly. I have no idea if the changes you have made make sense, or are
safe, or do the same thing that they did before. For example you change
getClassPath so that it could throw an exception. Is this safe? I don't know.
Someone else may, but I can't help review these types of Java functionality
changes.

    _______________________________________________________

Reply to this item at:

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

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