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

Defining width & height of a shape don't return any error if width & height were equal to 0 #555

Merged
merged 3 commits into from
Oct 15, 2019

Conversation

surger
Copy link
Contributor

@surger surger commented Aug 27, 2019

Add zero check before divizion

@Progi1984
Copy link
Member

Hi @surger, thank you for your contribution. Could you update the CHANGELOG.md and add a test in tests/PhpPresentation/Tests/Shape/AbstractGraphicTest.php before merge ? Thanks

@surger
Copy link
Contributor Author

surger commented Oct 10, 2019

@Progi1984 Which version should I specify CHANGELOG.md? And my commit so little, you may miss my commit :).
And I don't think what my changes need test.
AbstractGraphicTest.php have enough tests for it.

@Progi1984
Copy link
Member

@surger The next version is 0.10. No, each commit, each contribution is a part of next version.

Ok for no tests. You're right.

@Progi1984 Progi1984 added this to the 0.10.0 milestone Oct 10, 2019
@Progi1984
Copy link
Member

@surger Could you apply the same logic to the method setWidth ? And update Changelog.md 😉 After that I will merge it.

Add zero check before divizion
Add zero check before divizion
@Progi1984 Progi1984 changed the title Check if $this->height is zero Defining width & height of a shape don't return any error if width & height were equal to 0 Oct 14, 2019
@Progi1984 Progi1984 merged commit 344c779 into PHPOffice:develop Oct 15, 2019
@Progi1984
Copy link
Member

Thank you @surger for your contribution.

@surger surger deleted the patch-1 branch December 6, 2019 14:02
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