Support PHP 8.0

[craig410]

No description provided.

[acoulton]

Looks good, couple of BC breaks on method signatures tho (and off the top of my head I'm not certain what the inheritance/type-compat rules are for adding types to protected properties).

Might be best to ship as a 2.0, as a breaking-but-probably-not-really-breaking release, just in case anyone's extended any of those classes?

[craig410]

Yeah I know it's breaking. Should have said or created and targeted the new branch first. Seems might as well utilise the benefits of supporting the newer versions.

…

On Tue, 12 Oct 2021, 13:20 Andrew Coulton, ***@***.***> wrote: ***@***.**** commented on this pull request. Looks good, couple of BC breaks on method signatures tho (and off the top of my head I'm not certain what the inheritance/type-compat rules are for adding types to protected properties). Might be best to ship as a 2.0, as a breaking-but-probably-not-really-breaking release, just in case anyone's extended any of those classes? ------------------------------ In src/FestivalsApiClient.php <#5 (comment)> : > * @throws FestivalsApiClientException */ - protected function handleApiError($e): void + protected function handleApiError(BadResponseException $e): void I think this is a BC break, since the method is protected? ------------------------------ In src/FestivalsApiClientException.php <#5 (comment)> : > parent::__construct($message, $code, $previous_exception); $this->url = $url; } - /** - * @param int $code - * @param Exception|null $previous_exception - * - * @return FestivalsApiClientException - */ - public static function invalidJsonResponse(int $code, $previous_exception = NULL): FestivalsApiClientException + public static function invalidJsonResponse(int $code, ?Exception $previous_exception = NULL): FestivalsApiClientException Also a BC break? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAI2KG242WIDFFD6F5CLK7LUGQR2HANCNFSM5F2CCNXA> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.

[acoulton]

100% agree, breaking release it is.

[craig410]

Equally none of the breaking changes are required to support php 8 so I could release it as a minor and put the breaks through as a separate pull and release and then you have the choice as a user

…

On Tue, 12 Oct 2021, 13:50 Craig Gosman, ***@***.***> wrote: Yeah I know it's breaking. Should have said or created and targeted the new branch first. Seems might as well utilise the benefits of supporting the newer versions. On Tue, 12 Oct 2021, 13:20 Andrew Coulton, ***@***.***> wrote: > ***@***.**** commented on this pull request. > > Looks good, couple of BC breaks on method signatures tho (and off the top > of my head I'm not certain what the inheritance/type-compat rules are for > adding types to protected properties). > > Might be best to ship as a 2.0, as a > breaking-but-probably-not-really-breaking release, just in case anyone's > extended any of those classes? > ------------------------------ > > In src/FestivalsApiClient.php > <#5 (comment)> > : > > > * @throws FestivalsApiClientException > */ > - protected function handleApiError($e): void > + protected function handleApiError(BadResponseException $e): void > > I think this is a BC break, since the method is protected? > ------------------------------ > > In src/FestivalsApiClientException.php > <#5 (comment)> > : > > > parent::__construct($message, $code, $previous_exception); > $this->url = $url; > } > > - /** > - * @param int $code > - * @param Exception|null $previous_exception > - * > - * @return FestivalsApiClientException > - */ > - public static function invalidJsonResponse(int $code, $previous_exception = NULL): FestivalsApiClientException > + public static function invalidJsonResponse(int $code, ?Exception $previous_exception = NULL): FestivalsApiClientException > > Also a BC break? > > — > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub > <#5 (review)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAI2KG242WIDFFD6F5CLK7LUGQR2HANCNFSM5F2CCNXA> > . > Triage notifications on the go with GitHub Mobile for iOS > <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> > or Android > <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. > >

[acoulton]

Nah, not worth it when the changeset is so tiny I think.