Re: [VOTE] Apache POI 3.15 (RC1)

[Javen O'Neal]

While we're discussing enums backwards compatibility, are there any other
changes as part of https://bz.apache.org/bugzilla/show_bug.cgi?id=59836
that we absolutely cannot break backwards compatibility and blocks 3.16?

58190 : PictureType
(XSLF, scratchpad?)

58636 : AnchorType
(for drawings and comments, SS Common)

58671 :
MissingCellPolicy (class->enum, SS Common)

59264 : BorderStyle
(SS Common)

59790 : FormulaType
(internal, SS Common)

** 59791 : CellType
(SS Common)

59833 :
FillPatternType (for cell background texture, SS Common)

59837 :
HorizontalAlignment and VerticalAlignment (cell style alignment for SS
Common)

59873 : HyperlinkType
(Common SS, SL)

On Aug 22, 2016 11:27, "Javen O'Neal"  wrote:

> Nick, could you republish the Javadocs, please?
>
> On Mon, Aug 22, 2016 at 11:04 AM, Javen O'Neal  wrote:
> >>switch (cell.getCellType()) {
> >>case Cell.CELL_TYPE_NUMERIC:
> >
> > I have fixed the backwards compatibility problem in r1757235. See [1]
> > for more info.
> >
> > My vote is -1. We should rebuild the RC with r1757235.
> >
> > I have added this test [2] to prove that the old way of working with
> > cell types works without modification alongside the new way.
> >
> > [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=59791#c10
> > [2] https://svn.apache.org/viewvc/poi/trunk/src/testcases/org/ap
> ache/poi/ss/usermodel/BaseTestCell.java?r1=1757235=175723
> 4=1757235
> >
> > On Mon, Aug 22, 2016 at 8:30 AM, Dominik Stadler 
> wrote:
> >> Hi,
> >>
> >> Oh, sorry for the false alarm, this seems to be a problem with my
> >> compare-tool (Beyond Compare), it displays two test-documents as being
> >> contained on top-level of the src-tar.gz. When extracting via other
> tools I
> >> don't see those.
> >>
> >> So I am +1 here!
> >>
> >> However I saw that we do actually break some stuff with all the
> >> Enum-rework, e.g. the following will not work any more out of the box:
> >>
> >> switch (cell.getCellType()) {
> >> case Cell.CELL_TYPE_NUMERIC:
> >>
> >> Unfortunately a switch on teh CellType is quite common, so we need to
> >> explain this in the release notes and I think we should be a bit more
> >> conservative with all those refactorings/deprecations in the future to
> not
> >> cause too much change in those places!
> >>
> >> Dominik.
> >>
> >>
> >> On Mon, Aug 22, 2016 at 11:10 AM, David North 
> wrote:
> >>
> >>> Your image didn't come through. I see the following inside the poi-3.15
> >>> directory of poi-src-3.15-20160828.zip:
> >>>
> >>> legal
> >>> osgi
> >>> sonar
> >>> src
> >>> test-data
> >>> build.xml
> >>> forrest.properties
> >>> KEYS
> >>> LICENSE
> >>> NOTICE
> >>> patch.xml
> >>>
> >>> Which are unwanted? This seems to match the src packages in the last
> beta.
> >>>
> >>> Thanks,
> >>> David
> >>>
> >>> On 22/08/16 07:47, Dominik Stadler wrote:
> >>> > Sorry, me again,
> >>> >
> >>> > Unfortunately there are still some unwanted artifacts in the
> >>> > src-package, can you remove those as well?
> >>> >
> >>> >
> >>> > Inline image 1
> >>> >
> >>> > While not blocking the release, but the Java version used for the
> build
> >>> > seems to be "1.6.0_34", which is a bit outdated, latest (and last)
> >>> > version of Java 6 is patchlevel 45, would look better to use this
> one to
> >>> > build releases.
> >>> >
> >>> > Dominik.
> >>> >
> >>> >
> >>> > On Mon, Aug 22, 2016 at 12:49 AM, David North  >>> > > wrote:
> >>> >
> >>> > Vote begins now and ends at 11:55 BST 2016-08-23
> >>> >
> >>> > Artifacts are here:
> >>> >
> >>> > https://dist.apache.org/repos/dist/dev/poi/3.15-RC1/
> >>> > 
> >>> >
> >>> > Usual checks required (does it work? does the distribution look
> >>> right?),
> >>> > only more so as this is an RC for a non-beta release.
> >>> >
> >>> > +1 from me.
> >>> >
> >>> > As those of you watching the commits may have seen, I can't get
> the
> >>> svn
> >>> > commits from ant to work on either of my machines (Debian stable
> or
> >>> > Fedora 22) - I thought I'd fixed it by upgrading to svn 1.9, but
> >>> > seemingly not. I've therefore replaced them with "exec" tasks
> calling
> >>> > the SVN command line client. This needs further investigation.
> >>> >
> >>> > 

[Bug 59836] Tracker: Replace primitives with enums

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59836
Bug 59836 depends on bug 58190, which changed state.

Bug 58190 Summary: [PATCH] The current picture handling uses raw integers for 
types and index, replace with enum and reference
https://bz.apache.org/bugzilla/show_bug.cgi?id=58190

   What|Removed |Added

 Status|NEEDINFO|RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 58190] [PATCH] The current picture handling uses raw integers for types and index, replace with enum and reference

[bugzilla]

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

Javen O'Neal  changed:

   What|Removed |Added

 Status|NEEDINFO|RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

Re: [VOTE] Apache POI 3.15 (RC1)

[Nick Burch]

On Mon, 22 Aug 2016, Javen O'Neal wrote:

   switch (cell.getCellType()) {
   case Cell.CELL_TYPE_NUMERIC:


I have fixed the backwards compatibility problem in r1757235. See [1]
for more info.


Thanks! Some of this stuff can be a real pain :(

(Personally I figure I've still got another 1-2 years of faff to finish 
the POIFS -> OPOIFS, NPOIFS -> POIFS switch because of that stuff...)


Nick

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

Re: [VOTE] Apache POI 3.15 (RC1)

[Javen O'Neal]

Nick, could you republish the Javadocs, please?

On Mon, Aug 22, 2016 at 11:04 AM, Javen O'Neal  wrote:
>>switch (cell.getCellType()) {
>>case Cell.CELL_TYPE_NUMERIC:
>
> I have fixed the backwards compatibility problem in r1757235. See [1]
> for more info.
>
> My vote is -1. We should rebuild the RC with r1757235.
>
> I have added this test [2] to prove that the old way of working with
> cell types works without modification alongside the new way.
>
> [1] https://bz.apache.org/bugzilla/show_bug.cgi?id=59791#c10
> [2] 
> https://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java?r1=1757235=1757234=1757235
>
> On Mon, Aug 22, 2016 at 8:30 AM, Dominik Stadler  
> wrote:
>> Hi,
>>
>> Oh, sorry for the false alarm, this seems to be a problem with my
>> compare-tool (Beyond Compare), it displays two test-documents as being
>> contained on top-level of the src-tar.gz. When extracting via other tools I
>> don't see those.
>>
>> So I am +1 here!
>>
>> However I saw that we do actually break some stuff with all the
>> Enum-rework, e.g. the following will not work any more out of the box:
>>
>> switch (cell.getCellType()) {
>> case Cell.CELL_TYPE_NUMERIC:
>>
>> Unfortunately a switch on teh CellType is quite common, so we need to
>> explain this in the release notes and I think we should be a bit more
>> conservative with all those refactorings/deprecations in the future to not
>> cause too much change in those places!
>>
>> Dominik.
>>
>>
>> On Mon, Aug 22, 2016 at 11:10 AM, David North  wrote:
>>
>>> Your image didn't come through. I see the following inside the poi-3.15
>>> directory of poi-src-3.15-20160828.zip:
>>>
>>> legal
>>> osgi
>>> sonar
>>> src
>>> test-data
>>> build.xml
>>> forrest.properties
>>> KEYS
>>> LICENSE
>>> NOTICE
>>> patch.xml
>>>
>>> Which are unwanted? This seems to match the src packages in the last beta.
>>>
>>> Thanks,
>>> David
>>>
>>> On 22/08/16 07:47, Dominik Stadler wrote:
>>> > Sorry, me again,
>>> >
>>> > Unfortunately there are still some unwanted artifacts in the
>>> > src-package, can you remove those as well?
>>> >
>>> >
>>> > Inline image 1
>>> >
>>> > While not blocking the release, but the Java version used for the build
>>> > seems to be "1.6.0_34", which is a bit outdated, latest (and last)
>>> > version of Java 6 is patchlevel 45, would look better to use this one to
>>> > build releases.
>>> >
>>> > Dominik.
>>> >
>>> >
>>> > On Mon, Aug 22, 2016 at 12:49 AM, David North >> > > wrote:
>>> >
>>> > Vote begins now and ends at 11:55 BST 2016-08-23
>>> >
>>> > Artifacts are here:
>>> >
>>> > https://dist.apache.org/repos/dist/dev/poi/3.15-RC1/
>>> > 
>>> >
>>> > Usual checks required (does it work? does the distribution look
>>> right?),
>>> > only more so as this is an RC for a non-beta release.
>>> >
>>> > +1 from me.
>>> >
>>> > As those of you watching the commits may have seen, I can't get the
>>> svn
>>> > commits from ant to work on either of my machines (Debian stable or
>>> > Fedora 22) - I thought I'd fixed it by upgrading to svn 1.9, but
>>> > seemingly not. I've therefore replaced them with "exec" tasks calling
>>> > the SVN command line client. This needs further investigation.
>>> >
>>> > I also had to manually strip out the spurious "trunk" directory from
>>> the
>>> > src zip/tar, so that's another fix needed somewhere in the build
>>> > scripts.
>>> >
>>> > Thanks,
>>> >
>>> > --
>>> > David North | www.dnorth.net 
>>> >
>>> >
>>>
>>> --
>>> David North - Committer and PMC Member, Apache POI
>>> https://home.apache.org/~dnorth/
>>>
>>>

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 59836] Tracker: Replace primitives with enums

[bugzilla]

https://bz.apache.org/bugzilla/show_bug.cgi?id=59836
Bug 59836 depends on bug 59791, which changed state.

Bug 59791 Summary: Convert Cell Type to an enum
https://bz.apache.org/bugzilla/show_bug.cgi?id=59791

   What|Removed |Added

 Status|NEEDINFO|RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 59791] Convert Cell Type to an enum

[bugzilla]

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

Javen O'Neal  changed:

   What|Removed |Added

 Resolution|--- |FIXED
 Status|NEEDINFO|RESOLVED

--- Comment #12 from Javen O'Neal  ---
Added test for hard-coded int literals in r1757237

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

Re: [VOTE] Apache POI 3.15 (RC1)

[Javen O'Neal]

>switch (cell.getCellType()) {
>case Cell.CELL_TYPE_NUMERIC:

I have fixed the backwards compatibility problem in r1757235. See [1]
for more info.

My vote is -1. We should rebuild the RC with r1757235.

I have added this test [2] to prove that the old way of working with
cell types works without modification alongside the new way.

[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=59791#c10
[2] 
https://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/usermodel/BaseTestCell.java?r1=1757235=1757234=1757235

On Mon, Aug 22, 2016 at 8:30 AM, Dominik Stadler  wrote:
> Hi,
>
> Oh, sorry for the false alarm, this seems to be a problem with my
> compare-tool (Beyond Compare), it displays two test-documents as being
> contained on top-level of the src-tar.gz. When extracting via other tools I
> don't see those.
>
> So I am +1 here!
>
> However I saw that we do actually break some stuff with all the
> Enum-rework, e.g. the following will not work any more out of the box:
>
> switch (cell.getCellType()) {
> case Cell.CELL_TYPE_NUMERIC:
>
> Unfortunately a switch on teh CellType is quite common, so we need to
> explain this in the release notes and I think we should be a bit more
> conservative with all those refactorings/deprecations in the future to not
> cause too much change in those places!
>
> Dominik.
>
>
> On Mon, Aug 22, 2016 at 11:10 AM, David North  wrote:
>
>> Your image didn't come through. I see the following inside the poi-3.15
>> directory of poi-src-3.15-20160828.zip:
>>
>> legal
>> osgi
>> sonar
>> src
>> test-data
>> build.xml
>> forrest.properties
>> KEYS
>> LICENSE
>> NOTICE
>> patch.xml
>>
>> Which are unwanted? This seems to match the src packages in the last beta.
>>
>> Thanks,
>> David
>>
>> On 22/08/16 07:47, Dominik Stadler wrote:
>> > Sorry, me again,
>> >
>> > Unfortunately there are still some unwanted artifacts in the
>> > src-package, can you remove those as well?
>> >
>> >
>> > Inline image 1
>> >
>> > While not blocking the release, but the Java version used for the build
>> > seems to be "1.6.0_34", which is a bit outdated, latest (and last)
>> > version of Java 6 is patchlevel 45, would look better to use this one to
>> > build releases.
>> >
>> > Dominik.
>> >
>> >
>> > On Mon, Aug 22, 2016 at 12:49 AM, David North > > > wrote:
>> >
>> > Vote begins now and ends at 11:55 BST 2016-08-23
>> >
>> > Artifacts are here:
>> >
>> > https://dist.apache.org/repos/dist/dev/poi/3.15-RC1/
>> > 
>> >
>> > Usual checks required (does it work? does the distribution look
>> right?),
>> > only more so as this is an RC for a non-beta release.
>> >
>> > +1 from me.
>> >
>> > As those of you watching the commits may have seen, I can't get the
>> svn
>> > commits from ant to work on either of my machines (Debian stable or
>> > Fedora 22) - I thought I'd fixed it by upgrading to svn 1.9, but
>> > seemingly not. I've therefore replaced them with "exec" tasks calling
>> > the SVN command line client. This needs further investigation.
>> >
>> > I also had to manually strip out the spurious "trunk" directory from
>> the
>> > src zip/tar, so that's another fix needed somewhere in the build
>> > scripts.
>> >
>> > Thanks,
>> >
>> > --
>> > David North | www.dnorth.net 
>> >
>> >
>>
>> --
>> David North - Committer and PMC Member, Apache POI
>> https://home.apache.org/~dnorth/
>>
>>

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

Re: [VOTE] Apache POI 3.15 (RC1)

[Nick Burch]

On Mon, 22 Aug 2016, Javen O'Neal wrote:

Cell.CELL_TYPE_NUMERIC -> CellType.NUMERIC


We can't make a backwards incompatible change of this magnitude just like 
that. It would break almost every single user of POI's code, as well as 
breaking almost every example of using POI that's out there!


(Do a google search for "poi switch cell_type_numeric" and see how many 
hits you get!)



Aside from including this in the release notes, can we add some kind
of indicator on https://poi.apache.org/changes.html indicating that a
bug add/fix is a breaking change? This would save time for developers
reviewing the changelog and updating their code without needing to
read through every bug.


Anything that's a breaking change needs to be in a different area of the 
release notes


Nick

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 59791] Convert Cell Type to an enum

[bugzilla]

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

--- Comment #11 from Javen O'Neal  ---
Committed attachment 34170 from comment #10 in r1757235.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 59791] Convert Cell Type to an enum

[bugzilla]

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

--- Comment #10 from Javen O'Neal  ---
Created attachment 34170
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34170=edit
Improve backwards compatibility

This patch improves backwards compatibility (so that Cell.CELL_TYPE_* still
returns an int, works in a switch statement in Java 6, and has the same data
type as Cell.getCellType()).

Old usage:
int Cell.CELL_TYPE_*
int Cell.getCellType()
Cell.setCellType(int)

New usage:
CellType CellType.*
CellType Cell.getCellTypeEnum()
Cell.setCellType(CellType)

Both usages are supported with this patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60029] Problem with Days360 method for the month of february

[bugzilla]

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

--- Comment #5 from Javen O'Neal  ---
Created attachment 34169
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34169=edit
Failing unit test

I can confirm the described behavior is a bug in the latest trunk build
(r1757092, pre-3.16). See attached failing test case.

For reference, Microsoft's official documentation for this function is here:
https://support.office.com/en-us/article/DAYS360-function-B9A509FD-49EF-407E-94DF-0CBDA5718C2A

Days360.java:
https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/formula/functions/Days360.java
TestDays360:
https://svn.apache.org/viewvc/poi/trunk/src/testcases/org/apache/poi/ss/formula/functions/TestDays360.java

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

Re: [VOTE] Apache POI 3.15 (RC1)

[Dominik Stadler]

Hi,

Oh, sorry for the false alarm, this seems to be a problem with my
compare-tool (Beyond Compare), it displays two test-documents as being
contained on top-level of the src-tar.gz. When extracting via other tools I
don't see those.

So I am +1 here!

However I saw that we do actually break some stuff with all the
Enum-rework, e.g. the following will not work any more out of the box:

switch (cell.getCellType()) {
case Cell.CELL_TYPE_NUMERIC:

Unfortunately a switch on teh CellType is quite common, so we need to
explain this in the release notes and I think we should be a bit more
conservative with all those refactorings/deprecations in the future to not
cause too much change in those places!

Dominik.


On Mon, Aug 22, 2016 at 11:10 AM, David North  wrote:

> Your image didn't come through. I see the following inside the poi-3.15
> directory of poi-src-3.15-20160828.zip:
>
> legal
> osgi
> sonar
> src
> test-data
> build.xml
> forrest.properties
> KEYS
> LICENSE
> NOTICE
> patch.xml
>
> Which are unwanted? This seems to match the src packages in the last beta.
>
> Thanks,
> David
>
> On 22/08/16 07:47, Dominik Stadler wrote:
> > Sorry, me again,
> >
> > Unfortunately there are still some unwanted artifacts in the
> > src-package, can you remove those as well?
> >
> >
> > Inline image 1
> >
> > While not blocking the release, but the Java version used for the build
> > seems to be "1.6.0_34", which is a bit outdated, latest (and last)
> > version of Java 6 is patchlevel 45, would look better to use this one to
> > build releases.
> >
> > Dominik.
> >
> >
> > On Mon, Aug 22, 2016 at 12:49 AM, David North  > > wrote:
> >
> > Vote begins now and ends at 11:55 BST 2016-08-23
> >
> > Artifacts are here:
> >
> > https://dist.apache.org/repos/dist/dev/poi/3.15-RC1/
> > 
> >
> > Usual checks required (does it work? does the distribution look
> right?),
> > only more so as this is an RC for a non-beta release.
> >
> > +1 from me.
> >
> > As those of you watching the commits may have seen, I can't get the
> svn
> > commits from ant to work on either of my machines (Debian stable or
> > Fedora 22) - I thought I'd fixed it by upgrading to svn 1.9, but
> > seemingly not. I've therefore replaced them with "exec" tasks calling
> > the SVN command line client. This needs further investigation.
> >
> > I also had to manually strip out the spurious "trunk" directory from
> the
> > src zip/tar, so that's another fix needed somewhere in the build
> > scripts.
> >
> > Thanks,
> >
> > --
> > David North | www.dnorth.net 
> >
> >
>
> --
> David North - Committer and PMC Member, Apache POI
> https://home.apache.org/~dnorth/
>
>

[Bug 60029] Problem with Days360 method for the month of february

[bugzilla]

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

--- Comment #4 from Mark Murphy  ---
Ok, I see what you are looking at.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60029] Problem with Days360 method for the month of february

[bugzilla]

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

--- Comment #3 from Julien Houeix  ---
I get 30 with several versions of Excel, including Excel 2016. I attached a
screenshot.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60029] Problem with Days360 method for the month of february

[bugzilla]

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

--- Comment #2 from Julien Houeix  ---
Created attachment 34168
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=34168=edit
Result in Excel 2016

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org

[Bug 60029] Problem with Days360 method for the month of february

[bugzilla]

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

Mark Murphy  changed:

   What|Removed |Added

 OS||All

--- Comment #1 from Mark Murphy  ---
(In reply to Julien Houeix from comment #0)
> Hello,
> 
> There is a problem with the Days360 class in POI
> (org.apache.poi.ss.formula.functions.Days360.class).
> 
> The result of the evaluation is not the same as the Excel method when the
> start date is the last day of february and the end date is the last day of
> the month:
> Start date: 02/28/2018
> End date: 03/31/2018
> Result in Excel: 30
> Result in Apache POI: 31

I just tried this in Excel 2016, and it gave me 31. Maybe your build of Excel
has a bug? 31 is indeed the correct answer.

-- 
You are receiving this mail because:
You are the assignee for the bug.

-
To unsubscribe, e-mail: dev-unsubscr...@poi.apache.org
For additional commands, e-mail: dev-h...@poi.apache.org