Differential D3990

Numberstyles: Comment out not used format methods
ClosedPublic
Actions

Authored by danders on Jan 6 2017, 8:30 AM.

Details

Reviewers
staniek
mecir
boemann
Commits
R8:197ae032fb2b: Numberstyles: Comment out not used format methods
Summary

The numberstyles format methods are not used anywhere
and the corresponding unit test fails.

Afaics all number formatting is done in sheets atm.

Diff Detail

Repository
R8 Calligra
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
danders updated this revision to Diff 9792.Jan 6 2017, 8:30 AM
danders retitled this revision from to Numberstyles: Comment out not used format methods.
danders updated this object.
danders edited the test plan for this revision. (Show Details)
danders added reviewers: boemann, mecir.
Restricted Application added projects: KEXI, Calligra: 3.0. · View Herald TranscriptJan 6 2017, 8:30 AM
boemann edited edge metadata.Jan 6 2017, 8:37 AM

I doubt words will start using these indeed but still wouldn't they make sense for sheets, and if so i think it makes more sense for the tests to not fail

danders added a comment.Jan 6 2017, 8:50 AM

Yes, think so too, this is really just for 3.0 and mostly for disabling the failing test.
Sheets have exactly the same failing unit test, so planned to fix it there.

I think for 3.1 we should review odf to see if there are code in sheets (or elsewhere) that should be moved to libs.

boemann added a comment.Jan 6 2017, 8:51 AM

it has my blessing then but i'll let mecir give the final vote

staniek requested changes to this revision.Jan 6 2017, 10:15 PM
staniek added a reviewer: staniek.
staniek added a subscriber: staniek.

Disabling+a TODO comment is better than removing indeed. Even better would be #if 0 (better git-blame).

This revision now requires changes to proceed.Jan 6 2017, 10:15 PM
danders added a comment.Jan 9 2017, 11:34 AM

Hmm, I'm looking into sheets formatting and there is no unit test (my mistake) so I'm implementing that.

I'm thinking, maybe we should just XFAIL this test after all.
There is the bigger issue of how to solve number formatting in the future.
Why don't we use icu, or are we waiting for qt to pick it up, or.. ?

danders updated this revision to Diff 10320.Jan 18 2017, 1:31 PM
danders edited edge metadata.

Numberstyles: XFAIL scientific number unit tests

Keep all code even if it is not used anywhere

boemann accepted this revision.Jan 18 2017, 2:00 PM
boemann edited edge metadata.
Closed by commit R8:197ae032fb2b: Numberstyles: Comment out not used format methods (authored by danders). · Explain WhyJan 18 2017, 2:04 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 10322

View Options

libs/odf/KoOdfNumberStyles.cpp

Loading...
View Options

libs/odf/tests/TestNumberStyle.cpp

Loading...