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.

Sign up for GitHub

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

Jump to bottom

Rename classes, tags and functions #786

Merged
merged 34 commits into from
Jan 26, 2021
Merged

Rename classes, tags and functions #786

merged 34 commits into from
Jan 26, 2021

Conversation

murtukov
Copy link
Member

@murtukov murtukov commented Nov 30, 2020 •
edited
Loading

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

Implementation of #775

List of renames:

  • Class GlobalVariables -> GraphQLServices
  • Class ResolverResolver -> QueryResolver
  • Interface ResolverInterface -> QueryInterface
  • Service tag overblog_graphql.global_variable -> overblog_graphql.service
  • Service tag overblog_graphql.resolver -> overblog_graphql.query
  • Expression function resolver -> query

Also all UPGRADE-files for previous versions have been removed.

@murtukov murtukov requested a review from mcg-web November 30, 2020 19:10
@murtukov murtukov changed the title Refactor/global variables Rename classes, tags and functions Nov 30, 2020
mcg-web
mcg-web requested changes Dec 18, 2020
View reviewed changes
Copy link
Member

@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.

All these changes make sense but also make the migration from 0.13 to 1.0 a very heavy task. We should maybe introduce all these in a 0.14 version with deprecations so it ease the process.
What do you think of this ?

@murtukov
Copy link
Member Author

@mcg-web I think making 0.14 as a transition version is a good idea.

@murtukov murtukov mentioned this pull request Jan 11, 2021
Merged
@murtukov murtukov changed the title Rename classes, tags and functions [WIP] Rename classes, tags and functions Jan 11, 2021
@murtukov murtukov force-pushed the refactor/global-variables branch from 56e30ec to 138b119 Compare January 12, 2021 05:13
This was referenced Jan 13, 2021
Merged
Open
@murtukov murtukov force-pushed the refactor/global-variables branch from 5faf688 to f8716d7 Compare January 14, 2021 06:49
@murtukov murtukov changed the title [WIP] Rename classes, tags and functions Rename classes, tags and functions Jan 14, 2021
@murtukov murtukov requested a review from mcg-web January 14, 2021 07:12
@murtukov
Copy link
Member Author

murtukov commented Jan 14, 2021 •
edited
Loading

@mcg-web I've made it BC, it's ready to be merged. I also removed all UPGRADE files for previous versions. We can have a single UPGRADE.md file in each branch, like webonyx/graphql-php does.

@murtukov murtukov linked an issue Jan 14, 2021 that may be closed by this pull request
[RFC] Refactor GlobalVariables and resolver expressions #775
Closed
@murtukov
Merge branch 'master' into refactor/global-variables
7923316 
# Conflicts:
#	UPGRADE.md
#	docs/annotations/annotations-reference.md
#	tests/Config/Parser/AnnotationParserTest.php
#	tests/Config/Parser/fixtures/annotations/Type/Armored.php
#	tests/Config/Parser/fixtures/annotations/Type/Character.php
#	tests/Config/Parser/fixtures/annotations/Type/Planet.php
@murtukov murtukov requested a review from Vincz January 20, 2021 06:28
mcg-web
mcg-web requested changes Jan 20, 2021
View reviewed changes
Copy link
Member

@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.

We should also keep some legacy tests.

@murtukov
Add proper deprecation calls
9a1c272
@murtukov
Fix the signature check
f8728b9
@murtukov
Add tests for legacy code
b483a97 
* Make all GraphQL services available in the 'expression' constraint
* Remove code coverage suppressions
* Add GraphQLServices::getAll method
@murtukov
Copy link
Member Author

murtukov commented Jan 20, 2021 •
edited
Loading

@mcg-web

We should also keep some legacy tests.

I tweaked some tests to use legacy code, so there are no @codeCoverageIgnore tags required anymore

@murtukov murtukov requested a review from mcg-web January 22, 2021 14:10
@murtukov murtukov mentioned this pull request Jan 25, 2021
Closed
@murtukov
Copy link
Member Author

@mcg-web

I added some additional checks with reflection to decide, which signature should be used.

Here are some examples of which signature is used by certain calls:

  1. mutation('get_posts') - new signature
  2. mutation('get_posts', 'hello') - new signature
  3. mutation('get_posts', [args, 'test']):
    • If the array contains the same number of elements as the number of arguments in proxy resolver: old signature
    • Otherwise: new signature

@murtukov murtukov merged commit 4d0aaf9 into overblog:master Jan 26, 2021
@murtukov murtukov deleted the refactor/global-variables branch January 26, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vincz Vincz Vincz approved these changes

@mcg-web mcg-web mcg-web approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[RFC] Refactor GlobalVariables and resolver expressions
3 participants
@murtukov @Vincz @mcg-web