Skip to content

Feature/generator #682

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 24 commits into from
Jul 5, 2020
Merged

Feature/generator #682

merged 24 commits into from
Jul 5, 2020

Conversation

murtukov
Copy link
Contributor

@murtukov murtukov commented Jun 3, 2020

Q A
Bug fix? no
New feature? yes
BC breaks? yes
Deprecations? no
Tests pass? yes
Documented? no
Fixed tickets #668
License MIT

Finished integration of the new php code generator (PHPCodeGenerator). Documentation comes later.

The old php code generator is fully removed, there is no lib folder anymore. The new generator will be delivered as composer dependency.

IMPORTANT NOTE: there were unit tests inside lib/generator. Obviously this tests are now removed, since they were meant to test the library itself. I am not sure, if the bundle's tests cover all cases and if this will cause any negative consequences (@mcg-web please review this matter).

This is another big update, so I opened the PR as soon as 100% of the tests passed to reduce the changes of the PR. I still have many TODO's (which you can find in the code) to optimize and reduce the code of the new generator, but it will NOT have any crucial changes. Please, if you have time, pull my fork and test on your machine: https://github.com/murtukov/GraphQLBundle/tree/feature/generator (branch feature/generator)

Code style & static analysis:

This PR changes the php version to 7.4, which requires a new version of PHP-CS-Fixer, which in its turn applies new code styles. I temporarily commented this new rules in the .php_cs.dist and will uncomment them after this PR is merged and will open a separate PR for new code styles and static analysis changes.

Saving generated files

@mcg-web I am not sure about all write mode constants that are used by saving the contents into the files. Could you please look, if I used write modes correctly in the file TypeGenerator. One of the constants is never used, I think it’s MODE_DRY_RUN. I created a todo for you called “TODO (mcg-web)”

@murtukov murtukov requested review from Vincz, mcg-web and akomm June 3, 2020 02:09
@murtukov murtukov self-assigned this Jun 3, 2020
@murtukov murtukov linked an issue Jun 3, 2020 that may be closed by this pull request
@akomm
Copy link
Contributor

akomm commented Jun 4, 2020

Is >=7.4 not a bit steep? Don't get me wrong, I don't run age old PHP versions either. But currently most stuff is still 7.3 and it runs until 2022. I would probably get to 7.4 much sooner, but considering its public package, should we drop support so early?

Another, secondary thing I'v noticed:

Alpine has quite up to date packages and even bumping to the bleeding age versions would still give max. PHP 7.3.

This means, the docker test would probably not pass any more. Or it would require some manual fetching/building of php 7.4 instead of the packages.

While as long as it is possible to run the test without docker, I would not take it as a reason to block progress, but its still worth a consideration.

@murtukov
Copy link
Contributor Author

murtukov commented Jun 4, 2020

@akomm making GraphQLBundle 1.0 with PHP >= 7.4 is not my decision. It's stated in the requirements section.

Either way I can refactor code to PHP 7.3 if necessary.

@akomm
Copy link
Contributor

akomm commented Jun 4, 2020

If its applied at 1.0 and not before I think its not a problem. I assume its comming around the specified due november this year or later and not, like tomorrow. I think then 7.4 should be not a problem.

This was referenced Jun 27, 2020
@murtukov murtukov force-pushed the feature/generator branch from 7b17fa3 to da65c75 Compare June 29, 2020 19:08
murtukov added 16 commits June 30, 2020 02:21
- First working example with 100% passing tests
- Update PHP-CS-Fixer
- Update PHP-CS-Fixer for PHP7.4 support
- Add new rules to the .php_cs.dist but comment them until the PR is closed to avoid confusion
- Fix rendering of some closure expressions
- Remove a temporary test
- Remove PHP versions below 7.4
- Remove paths to old generator library
- Run php cs fixer
- Resolve conflict of PHP-CS-Fixer and PHPStan
- Change closures to arrow functions
- Replace hardcoded '$globalVariables' var with a dynamic one
- Replace hardcoded php-code with a generated one
@murtukov murtukov force-pushed the feature/generator branch from 932ae69 to abb6757 Compare June 30, 2020 00:39
@murtukov
Copy link
Contributor Author

@mcg-web I have cleaned commits, squashed and rebased. You can start review now 🚀

murtukov added 3 commits June 30, 2020 04:03
- Remove ExpressionProcessor.php
- Remove paths to old generator in phpunit.xml.dist
- Remove empty method
- Remove code related to old generator from composer.json
- Remove unused GraphQL test types
- Remove unused method getGlobalNames
@murtukov murtukov force-pushed the feature/generator branch from 5971b88 to 99792d1 Compare June 30, 2020 02:28
@murtukov murtukov force-pushed the feature/generator branch from 8f59278 to 8f9de7f Compare June 30, 2020 02:46
- Remove redundant code
- Change arg type
@murtukov murtukov force-pushed the feature/generator branch from 8f9de7f to 359b2d6 Compare June 30, 2020 02:55
@murtukov murtukov added this to the v1.0 milestone Jun 30, 2020
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.

Thanks @murtukov for this PR ! I have requested some minor changes so we could merge this.

@murtukov
Copy link
Contributor Author

murtukov commented Jul 5, 2020

@mcg-web done

@mcg-web mcg-web merged commit 0f2cbf9 into overblog:master Jul 5, 2020
@murtukov murtukov mentioned this pull request Jul 8, 2020
@murtukov murtukov deleted the feature/generator branch November 4, 2020 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New PHP code generator
3 participants