v1.5.0 contains breaking API changes
- Dominant language
- PHP
- Stars
- 3.6k
- Forks
- 773
- PR merge metrics
- No merged PRs in 30d
Description
Hi, first off - thanks for all the work that's been done recently to modernise the library.
However, I wanted to flag that the 1.5.0 release contains a number of breaking changes to the PHP API - and therefore I would have expected it to be 2.0.0.
These include:
* Changing the constructor signatures of the `CurlPost` and `SocketPost` classes in #598 (and deleting the wrapper classes that they previously took as their first argument). This will be a BC break for anyone who is creating either of those `RequestMethod` classes manually to pass into the `Recaptcha` constructor if they are passing arguments.
* Adding strict return types to methods throughout in 132d9a5 - including for the `RequestMethod` interface. These will be a BC break for anyone who has extended those methods or implemented that interface, as PHP will now enforce that the child method must have a strict return type that is compatible with the return type of the parent.
* Making non-final classes `readonly` in #618 - this will be a BC break for anyone who had extended that class (either for actual use, or for tests) because a readonly class can only be extended by a readonly class, and vice-versa.
Some of these may be more theoretical issues, or edge cases that only affect people using the library in unexpected ways. However, none of the affected code is e.g. marked as `@internal` or otherwise defined as not being part of the public API.
In particular, [ARCHITECTURE.md](https://github.com/google/recaptcha/blob/main/ARCHITECTURE.md) specifically says that end-users can create their own implementations of `RequestMethod` - these will be broken in 1.5.0 as above (unless the user-implementation already has a strict return type).
**Environment**
All environments.
**Proposed resolution**
The safest approach would be to:
* publish a new `2.0` based on the current `1.5.0`
* then revert the changes that went into `1.5.0` (either just the breaking ones, or all of them) and publish a new `1.5.1`.
It might also be a good opportunity to document what is / isn't expected to keep backwards compatibility as part of the public API in future?
Thanks
Contributor guide
Assessment
This issue has not been assessed yet.