Skip to content
New issue

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

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

Already on GitHub? # to your account

Add(linting) Add code linting to GitHub workflow, use php-cs-fixer #283

Merged
merged 2 commits into from
Feb 22, 2023

Conversation

thePanz
Copy link
Member

@thePanz thePanz commented Feb 15, 2023

Fixing the php-cs-fixer is not done, as it would generate tons of changes

@thirsch @connorhu : WDYT of those sets?

Unfortunately the array syntax must be "long" as we support php v5.3 → would it make sense to only support v5.4 onward?

@thePanz thePanz force-pushed the feature/add-php-cs-fixer branch 3 times, most recently from 1aa4aa1 to add19ae Compare February 15, 2023 23:01
@connorhu
Copy link
Collaborator

#261 (comment)

Based on this table we can drop 5.3 and mark 5.4 as the minimum at next version.

My opinion is that everything should be dropped below 5.6.

@thirsch
Copy link
Collaborator

thirsch commented Feb 16, 2023

From my point of view, every unsupported version could be dropped, as already mentioned in the linked issue and due to the fact, that we are only adding support for newer php versions. One could simply use an older version of sf1, if php 5.x is a must.

Edit: If the next release with any other platform requirement is versioned as 1.6, we could supply a fix for 1.5 in the rare case we are doing any bugfixing at all.

@thePanz
Copy link
Member Author

thePanz commented Feb 17, 2023

I would then propose to:

  1. apply the code changes (keeping the long array syntax)
  2. tag a latest release with 1.5.x → this will be the last release supporting php v5.x
  3. create a branch 1.5.x out of master
  4. rename the master branch to 1.6.x
  5. Remove php v5.x from the supported versions
  6. apply the php-cs-fixer introducing the short-array syntax (this way both branches are "aligned" on the coding styles)

I suggest to keep php v7.4 as min version to still support OS not yet shipping with php 8.x: this would allow a painless upgrade for those servers, while gaining some new language features

WDYT @thirsch @connorhu ?

@thirsch
Copy link
Collaborator

thirsch commented Feb 17, 2023

I would then propose to:

  1. apply the code changes (keeping the long array syntax)
  2. tag a latest release with 1.5.x → this will be the last release supporting php v5.x
  3. create a branch 1.5.x out of master
  4. rename the master branch to 1.6.x
  5. Remove php v5.x from the supported versions
  6. apply the php-cs-fixer introducing the short-array syntax (this way both branches are "aligned" on the coding styles)

I suggest to keep php v7.4 as min version to still support OS not yet shipping with php 8.x: this would allow a painless upgrade for those servers, while gaining some new language features

WDYT @thirsch @connorhu ?

I'd keep master (or rename it to main) instead of a version name as the current working branch (step 3), but I agree with all other steps.

@connorhu
Copy link
Collaborator

I would then propose to:

  1. apply the code changes (keeping the long array syntax)
  2. tag a latest release with 1.5.x → this will be the last release supporting php v5.x
  3. create a branch 1.5.x out of master
  4. rename the master branch to 1.6.x
  5. Remove php v5.x from the supported versions
  6. apply the php-cs-fixer introducing the short-array syntax (this way both branches are "aligned" on the coding styles)

I suggest to keep php v7.4 as min version to still support OS not yet shipping with php 8.x: this would allow a painless upgrade for those servers, while gaining some new language features
WDYT @thirsch @connorhu ?

I'd keep master (or rename it to main) instead of a version name as the current working branch (step 3), but I agree with all other steps.

I'd keep the master (or main) branch too, but the other steps looks good to me. If you specify the minimum version for the 1.6.x branch I can go with that in my Next Big Thing branch (spoiler: master...connorhu:symfony1:feature/phpunit )

@thePanz
Copy link
Member Author

thePanz commented Feb 17, 2023

I'd keep the master (or main) branch too, but the other steps looks good to me. If you specify the minimum version for the 1.6.x branch I can go with that in my Next Big Thing branch (spoiler: master...connorhu:symfony1:feature/phpunit )

Wow, that's impressive! 👍
I guess my try to introduce phpstan and rector should wait a bit :)

@thePanz thePanz force-pushed the feature/add-php-cs-fixer branch from cd84b18 to f364cfa Compare February 20, 2023 22:07
@connorhu
Copy link
Collaborator

@thePanz If you change the indent size in files (which is very fine by me) you would change it in editor config:
https://github.com/FriendsOfSymfony1/symfony1/blob/master/.editorconfig#L5

@thePanz
Copy link
Member Author

thePanz commented Feb 21, 2023

@thePanz If you change the indent size in files (which is very fine by me) you would change it in editor config: https://github.com/FriendsOfSymfony1/symfony1/blob/master/.editorconfig#L5

hum, that should be extended for different file extensions, right?

btw: I haven't got time to dig into the failures of the tests... any help there? 😊

@connorhu
Copy link
Collaborator

connorhu commented Feb 21, 2023

@thePanz If you change the indent size in files (which is very fine by me) you would change it in editor config: https://github.com/FriendsOfSymfony1/symfony1/blob/master/.editorconfig#L5

hum, that should be extended for different file extensions, right?

btw: I haven't got time to dig into the failures of the tests... any help there? 😊

There is the fix:
feature/add-php-cs-fixer...connorhu:symfony1:feature/add-php-cs-fixer

My browser is died under the huge change set so I can't create a review.

@thePanz thePanz force-pushed the feature/add-php-cs-fixer branch from f364cfa to 686a9cc Compare February 22, 2023 08:05
@thePanz thePanz force-pushed the feature/add-php-cs-fixer branch from 686a9cc to 39ff538 Compare February 22, 2023 08:06
@thePanz
Copy link
Member Author

thePanz commented Feb 22, 2023

@thePanz If you change the indent size in files (which is very fine by me) you would change it in editor config: https://github.com/FriendsOfSymfony1/symfony1/blob/master/.editorconfig#L5

There is the fix: feature/add-php-cs-fixer...connorhu:symfony1:feature/add-php-cs-fixer

Kosonom @connorhu !
Added the changes, and updated the .editorconfig setting to use 4 spaces

@thePanz thePanz merged commit 2cbd557 into master Feb 22, 2023
@thePanz thePanz deleted the feature/add-php-cs-fixer branch February 22, 2023 08:11
@thePanz thePanz mentioned this pull request Dec 15, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants