Skip to content
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

Fix OOM-Error in Printer::block #513

Merged
merged 3 commits into from
Dec 14, 2020
Merged

Conversation

webflo
Copy link
Contributor

@webflo webflo commented Dec 11, 2020

Issue #512

@webflo
Copy link
Contributor Author

webflo commented Dec 11, 2020

The formatting is a bit different from the original, but this only applies to small widths. It should even out on 80 and higher. 0 is a special case.

The formatting is hard to read. Maybe it makes sense to set it to 80? Not sure about the implications ...

@webflo
Copy link
Contributor Author

webflo commented Dec 12, 2020

Thought about it again, and removed the min in the constructor to use always the full terminal width. Added COLUMNS to the phpunit.xml fo ensure stable CI testing.

Copy link
Member

@theofidry theofidry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; could you apply make cs as well?

@webflo
Copy link
Contributor Author

webflo commented Dec 14, 2020

I did make cs but the diff was huge and it did not catch the wrong indention in my changes. 🤷

I fixed the files manually.

@theofidry
Copy link
Member

Yes I suppose. Requirement checker is a kind of library, but when we want to ship it in Box we need to ship an isolated version hence whenever there is a change of the requirement checker we "dump" it again (the isolated version). In itself it's a bit unnecessary noise during the course of a development, but it is necessary to dump it for a release. So you can do make cs and revert the dumped requirement checker changes without an issue

@theofidry theofidry merged commit 9548271 into box-project:master Dec 14, 2020
@theofidry
Copy link
Member

Thank you @webflo

@webflo webflo deleted the printer--block branch December 14, 2020 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants