Skip to content

Conversation

@justlevine
Copy link
Collaborator

What does this implement/fix? Explain your changes.

This PR fixes a bug in the Request::__construct() method, where the ->app_context was previously initialized twice.

Now it's only initialized once, while the $type_registry continues to be injected onto the AppContext class at the same point of the lifecycle.

Does this close any currently open issues?

Any other comments?

@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit fed16f8 and detected 0 issues on this pull request.

View more on Code Climate.

@justlevine justlevine added type: bug Issue that causes incorrect or unexpected behavior status: in review Awaiting review before merging or closing needs: reviewer response This needs the attention of a codeowner or maintainer scope: code quality Refactoring, linting, and enforcing coding standards labels Apr 26, 2025
@justlevine justlevine requested a review from jasonbahl April 26, 2025 01:11
@justlevine justlevine changed the title fix: don't reinitialize Request::$app_context in constructor fix: don't initialize Request::$app_context twice in class constructor Apr 26, 2025
@coveralls
Copy link

Coverage Status

coverage: 82.666% (-0.005%) from 82.671%
when pulling fed16f8 on justlevine:fix/dont-reinitialize-app-context
into e2c4442 on wp-graphql:develop.

@jasonbahl jasonbahl merged commit 1de7d0b into wp-graphql:develop Apr 28, 2025
37 of 39 checks passed
jasonbahl pushed a commit to justlevine/wp-graphql that referenced this pull request Apr 28, 2025
@justlevine justlevine deleted the fix/dont-reinitialize-app-context branch April 29, 2025 03:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs: reviewer response This needs the attention of a codeowner or maintainer scope: code quality Refactoring, linting, and enforcing coding standards status: in review Awaiting review before merging or closing type: bug Issue that causes incorrect or unexpected behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants