search

Crell / ApiProblem

A simple implementation of the api-problem specification. Includes PSR-15 support.
MIT License
239 stars 21 forks source link

Fixed type hints with strict types #23

Closed thomasvargiu closed 5 years ago

thomasvargiu commented 5 years ago

This PR fix type hint issues and enables strict types

Crell commented 5 years ago

Mm, no, we shouldn't be using nullables here. We should be setting reasonable type-safe defaults instead. Can you revise?

Crell commented 5 years ago

Resolves #22

thomasvargiu commented 5 years ago

I used nullable for backward compatibility with thre previous version. If we use a type everywhere it's a BC break so we should release a new major version.

Furthermore, to use types everywhere we should assign a default value to every property (it's not a problem, we can do it).

Probably we can consider the 3.0.0 version as a bugged one, so we can release this kind of changes in a 3.0.1 version.

What do you think?

Crell commented 5 years ago

3.0.1, with strict typing enabled and defaults added. No nullables. Let's do.

thomasvargiu commented 5 years ago

Done

thomasvargiu commented 5 years ago

Compete review needed.

I created another exception for json encoding exceptions. To centralize json exception messages I create an abstract exception.

Crell commented 5 years ago

Sorry about all the back and forth on this issue, but I'm actually really digging the improvements in error handling here. Thank you for digging in so much on this!

(Dare I ask what you're using this library for?)

Crell commented 5 years ago

Also, I think the changes here are significant enough that they warrant a 3.1 release, not a 3.0.1, whenever the next tag is.

thomasvargiu commented 5 years ago

Sorry about all the back and forth on this issue, but I'm actually really digging the improvements in error handling here. Thank you for digging in so much on this!

(Dare I ask what you're using this library for?)

It's not a problem, it's better to find out the best thing to do.

I'm updating a project, and a my ex collegue use it for APIs with zend-expressive. Before to update it using zendframework/zend-problem-details I prefer to refactor it still using this library to avoid big changes.

Also, I think the changes here are significant enough that they warrant a 3.1 release, not a 3.0.1, whenever the next tag is.

Yes, I agree.

Crell commented 5 years ago

Finally responded to the last open question here. Sorry for the holiday delay. :smile: Still planning to finish this up?