Skip to content

Conversation

@justlevine
Copy link
Contributor

@justlevine justlevine commented Jul 11, 2023

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 removes the usage of trailing commas, which throw a fatal error in PHP 7.2 and lower.

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?

Audited with WPGraphQL Coding Standards.

Note: This is not caught by PHPStan since our composer file is built on 7.3 for our dev-deps to work.

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

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 to me

@kidunot89 kidunot89 merged commit fa8e300 into wp-graphql:develop Jul 19, 2023
@justlevine justlevine deleted the fix/no-trailing-commas branch July 19, 2023 19:11
@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