Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for releases and dependencies management changes #4810

Closed
julienfalque opened this issue Feb 12, 2020 · 7 comments
Closed

Proposal for releases and dependencies management changes #4810

julienfalque opened this issue Feb 12, 2020 · 7 comments
Labels

Comments

@julienfalque
Copy link
Member

I'd like to suggest some changes in the way we maintain this project in hope we reduce the work load it implies.

Supported PHP versions

We still support PHP 5.6 which is EOL since more than a year now. I think we should only support PHP versions that are still actively maintained and continuously drop support of other versions. As of now that would mean supporting PHP 7.3 and 7.4 only. Users would be encouraged to upgrade their codebase to latest PHP version. When not possible, previous PHP CS Fixer versions remain available anyway.

Dropping support for a version of PHP should not be considered a BC break so we don't need a new major version of PHP CS Fixer to do so.

This would for example ease tests maintenance as we would need less version-specific test cases.

Dependencies

The dependencies of the tool use quite large version constraints, especially for Symfony components. This creates some frictions in our code as we need to e.g. support multiple method signatures, check method existence and so on.

Also, as I stated in #4560 (comment), this is mainly done because users tend to install PHP CS Fixer using the main composer.json of their projects, which leads to conflicts with version constraints there. When used as a tool, PHP CS Fixer's dependencies should be a hidden implementation detail and should never interfere with the codebase using it. When used as an actual dependency by package authors, the reasoning is the same as for PHP versions support: bumping a dependency version should not be considered a BC break.

I propose we strongly advocate not to install PHP CS Fixer in the composer.json of an application and assume people follow this advice so that we can manage our dependencies the way we actually need it, no the way users want it.

The main benefit of this would be a reduced maintenance burden when a new major version of some dependency is published.

Release cycle

As of now we have 4 release branches:

  • 2.15 (bugfixes, LTS);
  • 2.16 (bugfixes);
  • master (next minor);
  • 3.0 (next major).

To me, the main issue is the LTS branch: the more the codebase moves forward with new minor versions, the more it diverges from the LTS codebase and the more merges become difficult. Also we don't have any roadmap nor we provide any information about support period of our releases so having an LTS branch looks odd to me. I propose to drop it with no replacement to avoid that.

Another issue we face with most contributions is that it is often hard to decide that a specific change is a bug, an enhancement, a BC break or a new feature. We regularly decide that the nature of a submitted PR is different than what its author thought, which leads to changing PR's target and waste of time. I propose to drop the other bugfix branch too (currently 2.16):

  • master would always represent the next patch version by default, or the next minor version as soon as a new feature is merged;
  • we only maintain one bugfix branch at a time: when a new minor version is released, it means we stop releasing bugfix versions of the previous minor release, all bugfixes are released along with the new features in the same version.

The new branch workflow would be:

  • master (next minor if any new feature was merged, bugfixes otherwise);
  • 3.0 (next major).

All PRs would go to master unless introducing a BC break. This seems fairly easier for both maintainers and contributors. I would suggest switching branch names, i.e. 2.x for next patch/minor release (default branch) and master for next major, but that's not really important :)

@keradus
Copy link
Member

keradus commented Feb 13, 2020

Big 👎 from my side.

With only 2 branches, as you propose, as soon as we merge first feature to master branch, we are unable to do patch release. Then, lack of LTS is sth I would aim to avoid - big, proprietary projects cannot easily update, they are often staying with LTS only. We are not small repo that is almost not used by anyone, we should aim to maintain an LTS.

Dependencies conflicts when installing PHP CS Fixer in main composser.json of project willing to use it? We will still have composer.json file that can be conflicting, change of branching model won't change that. It is how composer works and for those who don't like it or simply don't want to use it we provide phar version (as-is and via PHIVE).

Merge conflicts? Very rare between 2.x LTS <> 2.y <> master. Often happening between master <> 3.0. Your branching model proposal won't change that.

@julienfalque
Copy link
Member Author

julienfalque commented Feb 13, 2020

With only 2 branches, as you propose, as soon as we merge first feature to master branch, we are unable to do patch release.

That's my point: the bugfixes would be included in the minor version. Not having a dedicated patch release for bugfixes is not a big deal: minor releases are supposed to remain compatible anyway. Maybe that would lead to more frequent minor releases if we have important bugfixes to publish, but that actually sounds like an improvement to me.

Then, lack of LTS is sth I would aim to avoid - big, proprietary projects cannot easily update, they are often staying with LTS only.

Not having an LTS does not mean those projects won't be able to run the tool. Old versions remain available. I bet there are a lot of projects that happily use old versions of PHP CS Fixer we stopped maintaining a while ago and it's fine.

We will still have composer.json file that can be conflicting, change of branching model won't change that.

I didn't say the branching model I propose would fix that, it's a different concern indeed. What I propose is that we stop having wide version constraints. If users have conflicts because they install the tool the wrong way, it should not be our problem which means we don't need to be compatible with e.g. Symfony 3.x.

Merge conflicts? Very rare between 2.x LTS <> 2.y <> master.

Indeed, but right now I'm trying to split BracesFixer into several little new fixers but I am facing an issue: as it introduces new rules, it should be merged into master. Then all BracesFixer bugfixes that will go into 2.15 will be complicated to merge into master because the patch from 2.15 will have to be dispatched into several other classes (maybe with new tests that won't pass despite everything was green on 2.15). Same reasoning applies to e.g. #4591.


You disagree with my new branching model proposal but what do you think about my proposal regarding PHP and dependencies versions?

@SpacePossum
Copy link
Contributor

Supported PHP versions

100% agree, however, having master on PHP7 will solve 99% of my issues

Dependencies

100% agree, users can still make a separate composer.json for their tools, but adding tools to the composer.json of the project is what causing the issues and should be very much discouraged

Release cycle

I still have to read this section :)

@kubawerlos
Copy link
Contributor

About the conflicts, I like what @ondrejmirtes did to PHPStan - separating it to distribution repo (0 dependencies) and source code repo.

@ondrejmirtes was it hard to make this split?

@keradus
Copy link
Member

keradus commented Mar 2, 2020

You disagree with my new branching model proposal but what do you think about my proposal regarding PHP and dependencies versions?

that's why i don't like having single thread for multiple topics, hard to manage for me :(

branching

minor releases are supposed to remain compatible anyway.

not true for ruleset. we have no BC promise for rulesets, and it hurt some of our users already. so instead of installing any MINOR, they install THE minor with any patch.

Solving versioning of rulesets is sth I would like to see happening, as it's blocking some of the user to update (or making update a harder, as now they need to adjust custom rules or not use @Symfony ruleset etc)

supported PHP versions

agree that it's not a bc breaker to bump PHP requirement, but which PHP engine to use since now on? imo we should bring numbers of "what our users are using" before doing any change here

dependencies

I would say it's up to the user, i keep tools myself installed as phar (via phive). i was thinking about adding a shim as well (i'm for it, just not enough time for do it recently)

@julienfalque
Copy link
Member Author

not true for ruleset. we have no BC promise for rulesets, and it hurt some of our users already. so instead of installing any MINOR, they install THE minor with any patch.

This should be the preferred way of installing the tool: somehow sticking to a specific (patch) version. This prevents CI from failing as soon as a new patch release e.g. changes some code style that wasn't fixed before because of a bug that is now fixed. What PHP CS Fixer fixes or not cannot be covered by a real BC policy as with public API. This is kind of what we currently do with PHPStan: we update it manually and fix new errors if any, but the CI is always green.

Anyway this does not require to maintain several versions at the same time, including an LTS version so I'm still in favor of dropping some branches.

Solving versioning of rulesets is sth I would like to see happening, as it's blocking some of the user to update (or making update a harder, as now they need to adjust custom rules or not use @Symfony ruleset etc)

To be honest I don't see the point. This was already discussed in various other threads: if you follow a ruleset, say @Symfony, then you're telling the tool you want to apply the same CS rules as the Symfony community does. If the CS rules of the Symfony community evolve, the ruleset can (must) evolve with it and still fulfill its contract which is "apply the CS rules of the Symfony community". Versionning of rulesets (and rules?) would be something really complicated to implement IMO (how many working versions of each rule will we have to maintain at the same time?) and will make even harder the regular discussion about "is this a feature, a bug, ...?" because we would also have to decide whether a change requires a new version of a rule/ruleset or which existing versions it should be merged into.

agree that it's not a bc breaker to bump PHP requirement, but which PHP engine to use since now on? imo we should bring numbers of "what our users are using" before doing any change here

I tend to have the opposite reasoning: how many projects don't upgrade to newer PHP versions because their ecosystem still provides maintained versions for so long? Dropping PHP versions sooner will likely encourage more projects to upgrade as the ecosystem moves forward. Old tags compatible with old PHP versions remain available anyway, they just won't receive bugfixes anymore. To me, numbers do not matter much, knowing that latest versions of PHP spread quite quickly is enough.

I would say it's up to the user, i keep tools myself installed as phar (via phive). i was thinking about adding a shim as well (i'm for it, just not enough time for do it recently)

Letting the user choose is the current situation that requires PHP CS Fixer to be compatible with as much Symfony versions as possible. I'm not saying we should forbid the scenario of installing PHP CS Fixer via the main composer.json but I do think we should consider it wrong instead of easing it.

Though having a shim version would definitely close this part of the discussion indeed :)

@keradus
Copy link
Member

keradus commented Feb 7, 2022

Hey,
I decided to close it as obsolete.
If you find any aspect still valid, please re-open or raise a new focused discussion.

Nowadays, we have single branch-versioning and shim package available

@keradus keradus closed this as completed Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants