Skip to content
This repository has been archived by the owner on Jul 17, 2024. It is now read-only.

Strict and native Types + PHP8.0 Drop + PHPUnit Upgrade [BC Break] #35

Merged
merged 8 commits into from
Jul 17, 2024

Conversation

marcelthole
Copy link
Contributor

Q A
BC Break yes

Description

As mentioned in #34 (comment) we could use the chance and add strict types and native types for the next release if we have already BC Breaks.

So here some (mostly automated) changes:

  • use `declare(strict_types=1);
  • Drop mcrypt support (was only for PHP 7)
  • Drop PHP 8.0 support
  • Upgrade PHPUnit to Version 10 and laminas-coding-standard to 2.5
  • Use native typehints for every method and property

What is missing?

  • use more strict array annotations, but these would not be a BC Break in the future
  • Maybe also introduce psalm? But that's more like an feature so i didn't added it here

Signed-off-by: Marcel Thole <[email protected]>
Signed-off-by: Marcel Thole <[email protected]>
@marcelthole marcelthole force-pushed the typesafe-upgrades branch 2 times, most recently from 44a26c7 to e601df1 Compare July 16, 2024 08:06
Signed-off-by: Marcel Thole <[email protected]>
Signed-off-by: Marcel Thole <[email protected]>
@Ocramius Ocramius requested a review from a team July 16, 2024 10:53
@Ocramius Ocramius added this to the 4.0.0 milestone Jul 16, 2024
@Ocramius
Copy link
Member

Note: we need to open 4.0.x before going for a merge here

Copy link
Member

@gsteel gsteel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me - I can't see any problematic changes other than the obvious BC breaks with native types. I'd approve if I was more familiar with this component, but I'll leave that to other members of the TSC 👍

Comment on lines -134 to -143
/**
* Get certificate string
*
* @return string
*/
public function getCertificate()
{
return $this->certificateString;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removal should probably be documented

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there something like a changelog or will it be manually filled with a release of a new tag?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just searched - There's no mention of it in any docs or any usage anywhere else, not even in tests.

As it's not mentioned in the docs, perhaps all that is required is a mention in the release notes. I don't think this is a big deal👍

Copy link
Member

@boesing boesing Jul 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tend to add stuff like this to the milestone manually using keep-a-changelog format.
did that a bunch of times for laminas-cache and related adapters.

https://github.com/laminas/laminas-cache/releases/tag/4.0.0

@Xerkus
Copy link
Member

Xerkus commented Jul 16, 2024

This component was voted to be abandoned but we failed to follow up and mark it as such. https://github.com/laminas/technical-steering-committee/blob/main/meetings/minutes/2023-11-06-TSC-Minutes.md#decisions

@gsteel gsteel changed the base branch from 3.13.x to 4.0.x July 17, 2024 16:24
@gsteel gsteel self-assigned this Jul 17, 2024
@gsteel gsteel merged commit 16ebe74 into laminas:4.0.x Jul 17, 2024
10 of 11 checks passed
@marcelthole marcelthole deleted the typesafe-upgrades branch July 18, 2024 05:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants