Skip to content

Conversation

@oleibman
Copy link
Collaborator

This is:

- [x] a bugfix (maintainability and coverage)
- [ ] a new feature

Checklist:

Why this change is needed?

PR #1580 defined constants for "stacked" alignment in cells.
Using those constants outside of Style/Alignment was beyond the
scope of the original PR, but I said I would get to it.
This PR replaces all uses of literal -165, and appropriate uses of
literal 255, with the named constants, and adds tests to make sure
that the changed code is covered in the test suite.

PR #1580 defined constants for "stacked" alignment in cells.
Using those constants outside of Style/Alignment was beyond the
scope of the original PR, but I said I would get to it.
This PR replaces all uses of literal -165, and appropriate uses of
literal 255, with the named constants, and adds tests to make sure
that the changed code is covered in the test suite.
@oleibman oleibman requested a review from MarkBaker February 1, 2021 18:16
@oleibman oleibman requested a review from MarkBaker February 2, 2021 17:00
@MarkBaker
Copy link
Member

Code is looking good now, thanks

I know in my more recent code, I'm making a lot more use of class constants, I definitely think it's a much more positive approach than literals, especially when the constants are descriptive in their names

@MarkBaker MarkBaker merged commit 2fac9ee into PHPOffice:master Feb 3, 2021
BlackyTay pushed a commit to BlackyTay/PhpSpreadsheet that referenced this pull request Aug 8, 2025
…#1716)

* Stacked Alignment - Use Class Constant Rather than Literal

PR PHPOffice#1580 defined constants for "stacked" alignment in cells.
Using those constants outside of Style/Alignment was beyond the
scope of the original PR, but I said I would get to it.
This PR replaces all uses of literal -165, and appropriate uses of
literal 255, with the named constants, and adds tests to make sure
that the changed code is covered in the test suite.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants