Skip to content

Common project code upgrade #695

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

Merged
merged 2 commits into from
Jul 9, 2020

Conversation

murtukov
Copy link
Contributor

@murtukov murtukov commented Jul 6, 2020

General upgrade of the whole project's code.

Upgrade PHPStan to the latest version

  • Uninstall deprecated phpstan-shim
  • Remove all unmatched ignored errors from phpstan-baseline.neon (about 700-800 lines)
  • Rename phpstan.neon.dist (remove .dist suffix) since all contributors have to stick to a single configuration
  • Fix ~700 new errors

Upgrade PHPUnit to the latest version

  • Change deprecated test methods accordingly

  • Configure new PHP-CS-Fixer rules:

    • Remove all redundant rules, which are already included in Symfony set.
    • Add new rules (now all classes, functions and constants will be automatically shortened and imported. Global functions don't require leading slash anymore)

I also manually went through all files and changed/fixed/tweaked code, added type-hints wherever possible.

Upgrade webonyx/graphql-php to the latest version

  • Update tests accordingly

Change version ranges for Symfony packages:

Change requirements in composer.json for all Symfony packages from ^4.3 || ^5.0 to 4.4 - 5.1, which is equal to
>=4.4.0 <5.2. Also add 2 additional jobs in .travis.yml to execute tests with Symfony 5.0 and Symfony 5.1.

Note:

With the new PHPStan version errors can be ignored directly in the code with comments, e.g.:

$name = $this->container()->get('service'); // @phpstan-ignore-line

/** @phpstan-ignore-next-line */
$name = $this->container()->get('service');

@murtukov murtukov requested review from mcg-web, Vincz, akomm, mavimo and a team July 6, 2020 07:09
@murtukov murtukov changed the title Common project code upgrade [WIP] Common project code upgrade Jul 6, 2020
@murtukov murtukov force-pushed the common_project_code_upgrade branch from a1f365e to decb608 Compare July 7, 2020 01:01
@murtukov murtukov changed the title [WIP] Common project code upgrade Common project code upgrade Jul 7, 2020
@murtukov
Copy link
Contributor Author

murtukov commented Jul 7, 2020

@mcg-web This PR is ready to me merged. It would be nice if we merge it ASAP, because it affects almost all files in the project.

Copy link
Contributor

@mcg-web mcg-web left a comment

Choose a reason for hiding this comment

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

here a first review on 78/282 files. This PR is huge so it review can't be faster sorry.

@mcg-web
Copy link
Contributor

mcg-web commented Jul 7, 2020

Allowing webonyx/graphql-php to the latest version should be done from 0.11, 1.0 version will not be release before November this make this a very long run. Same for symfony 5.1.

@murtukov
Copy link
Contributor Author

murtukov commented Jul 7, 2020

Allowing webonyx/graphql-php to the latest version should be done from 0.11, 1.0 version will not be release before November this make this a very long run. Same for symfony 5.1.

I didn't quite understand. Do I need to change something?

@mcg-web
Copy link
Contributor

mcg-web commented Jul 7, 2020

@murtukov this mean that we should first allow this version of webonyx/graphql-php and Symfony 5.1 in 0.11 so this change should not be integrated directly to this branch.

@murtukov
Copy link
Contributor Author

murtukov commented Jul 7, 2020

@mcg-web

  1. Why is it important to change the versions in previous branches first? Does it brake something? What happens if we update versions first in master and then in previous versions?
  2. How do we allow Symfony 5.1 in 0.11 if it requires Symfony >= 3.1, <=4.3?
    And webonyx/graphql-php 0.13 requires PHP >=7.1. The maximum we can do is upgrade versions in GraphQLBundle 0.13:
Version PHP Symfony Support
1.0 >= 7.4 >= 4.4 DEV
0.13 >= 7.2 >= 4.3 Active support
0.12 >= 7.1 >= 3.4, <4.4 Active support
0.11 >= 5.6 >= 3.1, <=4.3 Active support

@mcg-web
Copy link
Contributor

mcg-web commented Jul 7, 2020

Active support means that we maintain the project until we stop supports. 0.11 already supports Symfony 5.*. This doc is outdated, the true reference is composer.json anyway.

@murtukov
Copy link
Contributor Author

murtukov commented Jul 7, 2020

@mcg-web what about the PHP requirements? webonyx/graphql-php 0.13 requires PHP >=7.1 and GraphQLBundle 0.11 requires PHP >=5.6

https://github.com/webonyx/graphql-php/blob/master/UPGRADE.md#upgrade-v012x--v013x

@mcg-web
Copy link
Contributor

mcg-web commented Jul 7, 2020

Composer choice between the version that match your php version. 0.11 doesn't force to use 0.13 if your php version is compatible you can use it. The bundle required php minimum 5.6 so 7.1 also answer to this need.

@murtukov
Copy link
Contributor Author

murtukov commented Jul 8, 2020

@mcg-web Ok, I understand now.

Will you update webonyx version in previous branches?

@mcg-web
Copy link
Contributor

mcg-web commented Jul 8, 2020

I started working on this from 0.11, missing 8 tests to make green.

@murtukov murtukov force-pushed the common_project_code_upgrade branch from 0c748f5 to 7fa9265 Compare July 8, 2020 15:23
Copy link
Contributor

@mcg-web mcg-web left a comment

Choose a reason for hiding this comment

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

This LGTM, can you re-organize your commits so we can merge this please ? Thanks for this PR 💪

- Upgrade PHPStan
  - Uninstall deprecated phpstan-shim
  - Remove all unmatched ignored errors from phpstan-baseline.neon
  - Rename phpstan.neon.dist (remove .dist suffix)

- Upgrade PHPUnit to the latest
  - Change deprecated test methods accordingly

- Update all Symfony version constraints to ^4.4 || ^5.0

- Configure new PHP-CS-Fixer rules:
- Remove all unnecessary annotations

Manually go through all files and change/fix/tweak code, add type-hints wherever possible.
@murtukov murtukov force-pushed the common_project_code_upgrade branch from 7fa9265 to 2a97706 Compare July 8, 2020 23:13
@murtukov
Copy link
Contributor Author

murtukov commented Jul 8, 2020

@mcg-web done 😎

@murtukov murtukov mentioned this pull request Jul 8, 2020
@murtukov murtukov removed request for mavimo, Vincz and akomm July 9, 2020 01:56
@murtukov murtukov merged commit 566fa3f into overblog:master Jul 9, 2020
@murtukov murtukov deleted the common_project_code_upgrade branch November 4, 2020 10:23
@deeky666
Copy link
Contributor

When can we expect to use the new webonyx/graphql-php Version? It has been nearly 7 months ago v14 was released... Thank you!

@mcg-web
Copy link
Contributor

mcg-web commented Jan 11, 2021

@deeky666 can you open an issue please ? So we can have some information on the version you using and give you the best answer to your question.

@murtukov
Copy link
Contributor Author

murtukov commented Jan 11, 2021

@mcg-web @deeky666

There is no need to open an issue. The master branch already uses v14. Just need to wait for the next release, after this PR is merged. Don't know if it's planned to introduce v14 in previous versions of GraphQL Bundle.

@deeky666
Copy link
Contributor

deeky666 commented Jan 11, 2021

👍 thanks! I'll be watching the PR then.

@mcg-web
Copy link
Contributor

mcg-web commented Jan 11, 2021

0.14 will be released at the same time with 1.0 since it is just a transient version. 0.14 will have different php and symfony versions constraints, If these doesn't match your needs please open a dedicated issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants