fix stupid use of string for variable

Review Request #1824 - Created Feb. 24, 2015 and submitted

Information
David Turner
jmake
113ac68...
Reviewers
pants-reviews
benjyw, jsirois, zundel

This wasn't caught because the tests weren't run because the test file was misnamed; this RB also fixes the name of the test file.

The tests would sometimes fail because jmake would miss an update, and a fix for that is included here. There's also a fix for a (probably harmless but annoying) warning message.

ant test

Also, manual testing along with
https://rbcommons.com/s/twitter/r/1825/

Issues

  • 0
  • 0
  • 1
  • 1
Description From Last Updated
Stu Hood
Benjy Weinberger
Eric Ayers
David Turner
Review request changed

Status: Closed (submitted)

Nick Howard (Twitter)

This is a good find. I've got a couple comments.

src/com/sun/tools/jmake/PCDManager.java (Diff revision 1)
 
 
 

I'm not asking for a fix in this patch, but I'm pretty sure that a null check after a new will always fail. Maybe we should remove it?

src/com/sun/tools/jmake/PCDManager.java (Diff revision 1)
 
 

I'm not sure, but I think this was cls < src because the code above it is doing the same thing but with the operands reversed: !(src <= cls). If you change it, they won't be equivalent.

Maybe we should change line 706 and 716 to compare in the same order. Then their relationship would be clearer.

  1. You are correct: line 708 is also wrong. I'll file a new RB to correct that.

    (accidentally posted this in the wrong place because rb's "reply" button does not work)

David Turner

   
src/com/sun/tools/jmake/PCDManager.java (Diff revision 1)
 
 

You are correct: line 708 is also wrong. I'll file a new RB to correct that.

Loading...