https://bz.apache.org/bugzilla/show_bug.cgi?id=59336
Javen O'Neal <[email protected]> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #4 from Javen O'Neal <[email protected]> --- Thanks for the patch! Applied with minor modification in r1739533 and r1739536. Updated changelog in r1739537. Minor modifications: TestCellUtil#setCellStyleProperties: assertEquals(styCnt1 + 1, styCnt2); For helpful error messages when a junit assertion fails, the order of the parameters should be: assertEquals(expected, actual) or assertEquals(message, expected, actual). In this case, we expect 1 additional style to be created (styCnt+1), and the actual is styCnt2 (wb.getNumCellStyles() called after the cell style is added to the style table). I'm not too worried about line width. The coding style for this project aims for 80-100 characters width. I interpret this as "go over this limit when it improves readability". With the ubiquity of widescreen monitors, I assume that most developers looking at the code have screen space for 160-240 characters. I reverted some of your line-width/whitespace changs when I felt it didn't significantly improve the readability of the code. https://poi.apache.org/guidelines.html#CodeStyle Indentation: prefer 4 spaces over tabs, but better to be consistent in the method or class if a different indentation scheme is used. I replaced some tabs with spaces to make them consistent with the rest of the file. When deprecating methods, try to write the deprecated method in terms of the non-deprecated method to reduce code duplication (in case there's a bug in the non-duplicated method that gets fixed, you want the deprecated method to also get fixed) and for reducing the total number of lines of code--even if this makes the method slightly more expensive by recomputing values. See the change I made to CellUtil#setFont(Cell, Workbook, Font). This also makes it easier for developers to see how to update their code to the non-deprecated method. To check your javadocs, run "ant javadocs" (runs in under 1 minute). This caught a copy-paste error with {@link #setFont(Cell, short)}. -- You are receiving this mail because: You are the assignee for the bug. --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
