Skip to content

Conversation

justlevine
Copy link
Contributor

Your checklist for this pull request

Thanks for sending a pull request! Please make sure you click the link above to view the contribution guidelines, then fill out the blanks below.

🚨Please review the guidelines for contributing to this repository.

  • Make sure you are making a pull request against the develop branch (left side). Also you should start your branch off our develop.
  • Make sure you are requesting to pull request from a topic/feature/bugfix/devops branch (right side). Don't pull request from your master!
  • Have you ensured/updated that CLI tests to extend coverage to any new logic. Learn how to modify the tests here.

What does this implement/fix? Explain your changes.

This PR cleans up various useless variables and unnecessary ternaries from the codebase.

Branch is based off of #763, which should be merged first.

Does this close any currently open issues?

Any relevant logs, error output, GraphiQL screenshots, etc?

(If it’s long, please paste to https://ghostbin.com/ and insert the link here.)

Any other comments?

Several unused variables remain in the codebase, where its unclear at a glance if their lack of use is intentional or a bug. They will be picked up by WPGraphQL Coding Standards and can be evaluated/removed/refactored separately.

Where has this been tested?

  • WooGraphQL Version: 0.14.1
  • WPGraphQL Version: 1.14.6
  • WordPress Version: 6.2.2
  • WooCommerce Version: 7.8.2

'disabled' => $enable_auth_urls_hardcoded ? true : false,
'sanitize_callback' => function( $value ) {
'disabled' => $enable_auth_urls_hardcoded,
'sanitize_callback' => static function( $value ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intentionally leaked to prevent merge conflicts with #764

$item = \WC()->cart->get_cart_item( $payload['key'] );

return $item;
'resolve' => static function ( $payload ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is intentionally leaked to prevent merge conflicts with #764

Copy link
Collaborator

@kidunot89 kidunot89 left a comment

Choose a reason for hiding this comment

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

Looks good

@kidunot89 kidunot89 force-pushed the fix/cleanup-useless-variables branch from 36ad4ba to 30d43fa Compare July 19, 2023 20:10
@kidunot89 kidunot89 merged commit 58fbce7 into wp-graphql:develop Jul 19, 2023
@justlevine justlevine deleted the fix/cleanup-useless-variables branch July 19, 2023 21:44
@justlevine justlevine restored the fix/cleanup-useless-variables branch July 19, 2023 21:44
@justlevine justlevine deleted the fix/cleanup-useless-variables branch July 19, 2023 21:45
@kidunot89 kidunot89 added the dev-ops PR resolves an issue or implements a feature related to the development process label Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev-ops PR resolves an issue or implements a feature related to the development process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants