-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor and unit test PHPWord_Style_Font #93
Conversation
@@ -40,7 +40,8 @@ class PHPWord | |||
{ | |||
|
|||
const DEFAULT_FONT_NAME = 'Arial'; | |||
const DEFAULT_FONT_SIZE = 20; | |||
const DEFAULT_FONT_SIZE = 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you change again 😉 the default size ?
The "old" default font size was defined in halfpoints. Now we define it in points. It's the same: 10 (points) * 2 = 20 (halfpoints). |
Could you add a comment to the constant for understanding the unit of the const ? Thanks ;) |
Ok. Done. Thanks for reminding. |
I merge it now :) |
Refactor and unit test PHPWord_Style_Font
Thanks. Should I write the refactoring to the changelog? |
As you want for this ;) or add it in the next pull request :) |
I'll add it to the next request :) By the way, do we want to remove the underscore prefix for private properties/methods name on this version? I see that's one of the source of error reported by Travis. |
As you want, but the first step stays unit tests and issues fixing. |
Ok. I'll stay with unit tests and issues fixing. |
For consistency, we will use points instead of halfpoints. The size will be converted to halfpoints during XML writing. I've checked and corrected all usages. I also introduce DEFAULT_FONT_COLOR constant instead of hardcoded '000000'.
PHP_Style_Font is now in 100% coverage.