search

WordPress / phpunit-test-reporter

GNU General Public License v3.0
29 stars 21 forks source link

Change `commit` field type from `numeric` to `number`. #93

Closed getsource closed 4 years ago

getsource commented 4 years ago

While validation is happening due to the validation callback, we started getting REST API deprecation notices because numeric is not a valid built-in type.

See: https://core.trac.wordpress.org/browser/tags/5.5.1/src/wp-includes/rest-api.php#L1583

This was causing the tests to fail.

This PR proposes changing to a type of number and letting WP's validation handle checking whether the value is numeric.

pfefferle commented 4 years ago

@getsource should we upmerge the latest changes, to see if all travis test are green?

getsource commented 4 years ago

@pfefferle I brought the PR up to date with the changes you merged, and things are passing as expected now.

@danielbachhuber Do you know if there was a specific reason not to use core's validation here? I'm guessing it's possible things have changed since, too, but wanted to be sure I understood as much as possible before removing validation code.

getsource commented 4 years ago

On further looking, should we go ahead and set this to integer rather than number, since that's more specific?

pfefferle commented 4 years ago

Since it is always an integer, I would go with the more specific!

danielbachhuber commented 4 years ago

Do you know if there was a specific reason not to use core's validation here? I'm guessing it's possible things have changed since, too, but wanted to be sure I understood as much as possible before removing validation code.

I don't recall the reasoning, no. It might've been that the validation code hadn't landed in core yet. In any case, seems fine to change.

getsource commented 4 years ago

Thanks so much!