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.

Sign up for GitHub

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

Jump to bottom

Fix Float validator that doesn't know how to handle an int #61

Closed
wants to merge 1 commit into from
Closed

Fix Float validator that doesn't know how to handle an int #61

wants to merge 1 commit into from

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Mar 1, 2021 •
edited
Loading

Ports fix from Shardj/zf1-future#135

Author comments (from Shardj/zf1-future#135 (comment)):

Looks like whoever wrote Float.php didn't pay much attention to ints.

@glensc
Copy link
Contributor Author

glensc commented Mar 1, 2021

@falkenhawk please validate the change before merging, I only copied the change over from the other repo to keep the repos differences down.

@glensc glensc changed the title Fix validator that doesn't know how to handle an int Fix Float validator that doesn't know how to handle an int Mar 1, 2021
@Shardj @glensc
Fix Float validator that doesn't know how to handle an int
a038854 
Refs:
- Shardj/zf1-future#135

Signed-off-by: Shardj <me@georgeappleton.co.uk>
Signed-off-by: Elan Ruusamäe <glen@pld-linux.org>
@glensc
Copy link
Contributor Author

glensc commented Mar 3, 2021

Tests fail now, so this is apparently not complete, back to Draft:

There were 2 failures:

1) Zend_Validate_FloatTest::testBasic
Failed asserting that false matches expected true.

/home/runner/work/zf1/zf1/tests/Zend/Validate/FloatTest.php:92

2) Zend_Validate_FloatTest::testNoZendLocaleButPhpLocale
Failed asserting that false is true.

/home/runner/work/zf1/zf1/tests/Zend/Validate/FloatTest.php:141

@glensc glensc marked this pull request as draft March 3, 2021 08:06
@michaelcaldwell-st
Copy link

@glensc any update on this? What are the assertions that are failing? Are they dependent on actually incorrect behavior? This fix would still be helpful to us. Thanks!

@glensc
Copy link
Contributor Author

glensc commented Mar 12, 2021

@michaelcaldwell-st no progress. you can jump in and look at the code yourself. the error includes files with line numnbers.

@falkenhawk
Copy link
Member

@glensc I believe this has been fixed already in #16. Same story as #100 (comment)

@falkenhawk falkenhawk closed this Sep 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelcaldwell-st michaelcaldwell-st michaelcaldwell-st approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@glensc @michaelcaldwell-st @falkenhawk @Shardj