Create a new major release for every Laravel style change
lhsazevedo opened this issue · 12 comments
Since Laravel 6 is adopting a different code styling from its predecessor, it would be interesting to use different versions for each of them - 1.x for Laravel 5 and 2.x for Laravel 6, for example.
This is an interesting idea. Is Laravel continuing to evolve the code style on previous versions or are they only changing the style for new versions?
The way I've been handling this so far is to merge changes as Laravel does, so for example if you were on 5.x and wanted to match the framework's style exactly you would need to use <= 0.4.0 as the version constraint for this package. Since we haven't hit a stable release yet and these are usually breaking changes I've been incrementing the minor version.
My primary concern with incrementing our major version number when Laravel does is there might not be any breaking changes in a major Laravel release, but there might be breaking changes in a Laravel patch release.
For example, there might not be any 'breaking' changes to the code style between Laravel 6 and 7 but there might be a breaking change in the code style between Laravel 6.3.1 and Laravel 6.3.2.
I don't think Laravel considers changing the code style a breaking change but for this package I would consider it a breaking change since it can break your build.
So we might need to continue making 'major' releases (which are minor releases until we do a 1.0 release) when the code style actually changes. Maybe we can add a table to the README that helps users determine which version of this package to use for a given Laravel version?
It would also be good to clarify what constitutes a breaking change for this library. It's not really documented atm and it's a little different than a normal PHP library.
It seems to me that they treat style changes as breaking, but it's just a presumption we can't rely on.
Making major releases when Laravel changes the style seems to be the best path, something like this:
| Laravel | Code Style |
|---|---|
| 5.x | 0.4.x |
| 6.0 - 6.3.1 | 0.5.x |
| >= 6.3.2 | 0.6.x |
First step would be create two new branchs: 0.4 at a4fb38b and 0.5 at addc0fe
I tracked down the last style change and it looks like it was made in 6.0.3.
https://github.com/laravel/framework/blob/6.x/CHANGELOG-6.x.md#refactoring-1
laravel/framework@1bbe552
If that's the case I don't think we should create major releases for each Laravel release for the reasons outlined above but it would be great to find some other way to provide guidance re: Laravel versions <> laravel-code-style versions.
Yup, that's exactly what I meant: a major release on every style change. The table was just an illustration to the hypothetical non-major 6.3.2 style change that you used in an earlier comment as example.
So the table for the different styles we have today (based on the release you tracked down) would be:
| Laravel | laravel-code-style |
|---|---|
| 5.0 - 6.0.2 | 0.4 |
| >= 6.0.3 | 0.5 |
Will a new release be created for the updates to Alpha Ordered Imports soon? 🙂 Anything I can help with? 👍 I agree that a minor release (as per 0.x formatting) sounds good. 👌
Regarding #16, is that necessary as the standards for Laravel 5 probably won't be changing, so anyone using 5.x can require ^0.4, and anyone with 6.0.3 or later can use ^0.5?
@pxgamer You can require dev-master for now, Alpha Ordered Imports is already committed.
composer require --dev matt-allan/laravel-code-style@dev-master.
@SagarNaliyapara, try using composer require --dev matt-allan/laravel-code-style:dev-master instead. 👍 The @ is causing the issue.
Thanks mate, it works! @pxgamer
Hey everyone, sorry for the delay. I've tagged 0.5.0 with the alpha ordered imports change. I also added the table as @lhsazevedo suggested to the README.
https://github.com/matt-allan/laravel-code-style#releases
I think that covers everything so I'm going to close this but let me know if anything can be clarified. Thanks!
I've tagged 0.5.0 with the alpha ordered imports change
Much appreciated 🤗
I was pinning the package to master for this already, can confirm: the update worked => rebing/graphql-laravel#563 👍
Awesome!