Skip to content
This repository has been archived by the owner on Dec 3, 2023. It is now read-only.

breaking changes in non-major releases #4169

Closed
staabm opened this issue Jun 13, 2022 · 14 comments
Closed

breaking changes in non-major releases #4169

staabm opened this issue Jun 13, 2022 · 14 comments

Comments

@staabm
Copy link
Contributor

staabm commented Jun 13, 2022

with symplify 10.3.x a lot of classes and functionality was dropped, which manifests as a big BC break from the consumer point of view.

since we assumed symplify would adhere to semantic versioning, lots of our projects just upgraded from 10.2 to 10.3 and ran into errors because of that.

would be great if you could think about doing "proper" major releases when working BC breaks are involved. it would would ease upgrading for the project consumers and prevent angry people ;-).

I don't say you should not drop features and make BC breaks (thats your choice) but if you are doing so, please mark it as a new major version. thanks for considering.

@TomasVotruba
Copy link
Member

Thanks for feedback. To be honest, I lack time and resources to maintain these packages propperly. Rector and ECS take most of my OSS time so any help is appreciated :)

What do you suggest to do now?

@TomasVotruba
Copy link
Member

Could you be more specific about the BC breaks? So I know if there are some bugs left and what parts of Symplify is used more than I expected :)

@staabm
Copy link
Contributor Author

staabm commented Jun 13, 2022

To be honest, I lack time and resources to maintain these packages propperly

thats totally fine.

the only thing which "should have been done" differently is naming the new release symplify 11.x instead of 10.3, that way consumers which use composer constraints like symplify/symplify: "^10" won't run into problems.

Rector and ECS take most of my OSS time so any help is appreciated

I try to help where time allows :-)

What do you suggest to do now?

I don't think the problem can be fixed anymore, since people which used ^10 as a constraint already get the latest version with changes. I re-worked our constraints to 10.2.* for now, so we don't run into the 10.3 problems right now.

Could you be more specific about the BC breaks

e.g. dropping phpstan rules is a BC break, as people might have cherry-picked the rules and registered them in their phpstan configs.

another source of BC breaks is the path to e.g. phpstan- *.neon files. when you rename packages or move them arround, from a consumer point of view, we need to adjust the phpstan configs which include these files.

bugs like #4163 is not a BC concern/problem from my point of view.. these things happen and have been in the codebase for longer time.

So I know if there are some bugs left

the ones which bite us last week have been fixed, and since you are already pushed a release thats good enough for us.
thx for releasing that fast.

.. what parts of Symplify is used more than I expected :)

we mostly depend on symplify/phpstan-rules.

in the past we also depended on symplify/phpstan-extensions to use AbstractServiceAwareRuleTestCase. I removed this dependency though, because we now use PHPStan\Testing\RuleTestCase triggered by the deprecation you put in - thanks for that.

@staabm
Copy link
Contributor Author

staabm commented Jun 13, 2022

I just remember one BC break which I have forgotten in the previous comment.

we implemented custom rules which used things like Symplify\Astral\NodeFinder\SimpleNodeFinder which no longer exist.
would be great those classes could be re-introduced but just marked as @deprecated?

@TomasVotruba
Copy link
Member

TomasVotruba commented Jun 13, 2022

I just remember one BC break which I have forgotten in the previous comment.

we implemented custom rules which used things like Symplify\Astral\NodeFinder\SimpleNodeFinder which no longer exist.
would be great those classes could be re-introduced but just marked as @deprecated?

Could you send PR with adding it?
I can release the new patch to keep it.

Then 11.0 to separate it.

@TomasVotruba
Copy link
Member

another source of BC breaks is the path to e.g. phpstan- *.neon files. when you rename packages or move them arround, from a consumer point of view, we need to adjust the phpstan configs which include these files.

Understood. I never thought people are using whole sets of those :)

Could you add old BC paths that import the new ones?
Then I can include it in the patch version.

@staabm
Copy link
Contributor Author

staabm commented Jun 13, 2022

Could you send PR with adding it?
I can release the new patch to keep it.
Could you add old BC paths that import the new ones?

I will re-store what we need, thx for caring.

Understood. I never thought people are using whole sets of those :)

no worries - our use cases are a bit esoteric :-)

another point I see now: SimpleNodeFinder was declared as @api which made me feel it should not just disappear between non major releases.

@staabm
Copy link
Contributor Author

staabm commented Jun 13, 2022

just opened a issue on phpstan-nette, to report errors when @deprecated classes are referenced from nette config files.

this might help smoothen migrations paths

@staabm
Copy link
Contributor Author

staabm commented Jun 13, 2022

using the latest 10.3.3 release thanks to phpstan-deprecation-rules I am now seeing nice deprecation errors but the build stays green.

next step would be to release 11.0 (with the BC breaking changes included, we previously had in the 10.3 release) - so essentially you could just revert #4170

after these steps we will have a "clean" 11.0 release and a stable again 10.3 - so this issue can be closed IMO.

Thanks again for beeing open for this additional work

@staabm
Copy link
Contributor Author

staabm commented Jun 13, 2022

STOP. just notices one more failure:

The data provider specified for clxSymplify\PHPStanExtensions\Tests\ExtensionNodeScopeResolverTest::testFileAsserts is invalid.
_PHPStan_98a3b0791\Nette\FileNotFoundException: File '/home/runner/work/php-clx-symplify/php-clx-symplify/packages/phpstan-extensions/tests/config/../../../../vendor/symplify/phpstan-rules/packages/symfony/config/services.neon' is missing or is not readable.

will provide a fix

@staabm
Copy link
Contributor Author

staabm commented Jun 13, 2022

got it sorted on our end. I am fine with proceeding as described in #4169 (comment)

@TomasVotruba
Copy link
Member

All right :) the PR can be reverted with a button on GitHub. Could you send PR to remove the contents? I'll release the major then

@staabm
Copy link
Contributor Author

staabm commented Jun 23, 2022

thank you again

@staabm staabm closed this as completed Jun 23, 2022
@TomasVotruba
Copy link
Member

Glad to co-operate and thank you for exact plan and actions steps 👍

@deprecated-packages deprecated-packages locked as resolved and limited conversation to collaborators Oct 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

2 participants