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.

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

Use type names when possible #1423

Merged
merged 2 commits into from
Aug 8, 2023
Merged

Use type names when possible #1423

merged 2 commits into from
Aug 8, 2023

Conversation

ruudk
Copy link
Contributor

@ruudk ruudk commented Aug 8, 2023

Instead of calling Type::builtInTypes which calls Introspection::getTypes() that creates a lot of objects, we only need the type names.

I noticed this while working on something else and I think this is a good improvement.

Copy link
Collaborator

@spawnia spawnia left a comment

Choose a reason for hiding this comment

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

Great change.

src/Type/Definition/NamedTypeImplementation.php Outdated Show resolved Hide resolved
@ruudk ruudk force-pushed the typeless branch 4 times, most recently from 4024b7d to c988bb8 Compare August 8, 2023 07:27
@ruudk
Copy link
Contributor Author

ruudk commented Aug 8, 2023

@spawnia Although the php-cs-fixer bot is super nice, the fact that it runs as a GitHub Actions causes it to not trigger the CI workflows. It can only be done like that when you use a Personal Access Token.

Instead of calling Type::builtInTypes which calls Introspection::getTypes() that creates a lot
of objects, we only need the type names.
@ruudk
Copy link
Contributor Author

ruudk commented Aug 8, 2023

@spawnia It's green now!

@spawnia spawnia merged commit f6b13fc into webonyx:master Aug 8, 2023
@shmax
Copy link
Contributor

shmax commented Aug 8, 2023

Nice one. 👍

@ruudk ruudk deleted the typeless branch November 11, 2024 11:11
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