https://bz.apache.org/bugzilla/show_bug.cgi?id=58787

Javen O'Neal <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO

--- Comment #17 from Javen O'Neal <[email protected]> ---
(In reply to Mark Murphy from comment #15)
> Created attachment 33667 [details]
> Completed ant patch including tests, examples, and documentation

Attachment 33667 doesn't compile because it imports OOXML classes (appears to
only be for javadoc linking). OOXML classes aren't compiled or available to
org.apache.poi.ss.util. Replace these {@link} references with regular text
references and delete the imports and it'll be good. If you have code that
relies on OOXML classes, you have to write Voldemort code or take advantage of
inheritance of methods that are resolved at run-time [1]. If you want to check
that your code compiles (and passes unit tests) before generating the patch
file, run "ant test" or "ant clean test".

The solution we've accepted for junit tests is to either duplicate the test in
TestHSSFWorkbook, TestXSSFWorkbook, and TestSXSSFWorkbook, or write a single
test (preferred) in BaseTestWorkbook that uses testDataProvider.createWorkbook.
In the case of org.apache.poi.ss.util junit tests, we've usually picked
HSSFWorkbook as the class to test the methods (for the same OOXML availability
reason as above). I'd like to rearrange the org.apache.poi.ss.util tests to run
with HSSF, XSSF, and SXSSF instances to make sure they all work (either with
junit testcase parameterization or with ITestDataProvider as we have done for
org.apache.poi.hssf.usermodel, .xssf.usermodel, and .xssf.streaming unit
tests). Anything in org.apache.poi.ss.util should probably work on all 3 kinds
of workbooks, with the same behavior (unless a different behavior makes sense),
but this is outside the scope of this bug.

You mentioned in the javadocs that PropertyTemplate would replace RegionUtil. I
haven't compared the two classes side-by-side yet. Do you mean that
PropertyTemplate is a higher-level wrapper around RegionUtil, and that most
people using RegionUtil would likely be more interested in PropertyTemplate? Or
is PropertyTemplate a rewrite of RegionUtil, fixing inconsistencies in the
provided methods? I'm a fan of code reduction on my personal projects, but I
try to keep POI backwards compatible if at all possible, and retire features 2+
releases after announcing deprecation (if possible). Your patch didn't include
any changes to RegionUtil. What are your long-term plans for this class?
Deprecate it? Merge PropertyTemplate into RegionUtil (or vice versa, the less
backwards-compatible option of the two)? Call RegionUtil from PropertyTemplate?

Thanks for the hard work you've put into this bug.

[1] Using class inheritance rather than if-then statements to avoid
compile-time dependencies. Added TestDataProvider.createWorkbook(int
rowAccessSize) and TestDataProvider.trackAllColumnsForAutosizing, which don't
do anything special for HSSFWorkbook or XSSFWorkbook, but have special behavior
for SXSSFWorkbook: https://svn.apache.org/viewvc?view=revision&revision=1730997

-- 
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]

Reply via email to