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

PHP Code Review #40

Merged
merged 10 commits into from
Jan 26, 2021
Merged

PHP Code Review #40

merged 10 commits into from
Jan 26, 2021

Conversation

tfrommen
Copy link
Contributor

@tfrommen tfrommen commented Jan 7, 2021

This PR targets PHP-specific code improvements or changes to be made that have been mentioned in #38.

In more detail, these include:

  • Import all used classes from global namespace.
  • Remove leading backslash for imported global classes.
  • Import enqueue function from Asset Loader.
  • Use array_diff instead of array_filter.
  • Use query API functions instead of WP_Query.
  • Replace incorrectly used array_map calls.
  • Add constant for CURIE template.
  • Refactor filter logic for edit post type views, and prevent the "Mine" view from missing if there is no "All" view.
  • Introduce new get_author_ids() function.

This PR does not conflict with any of the other three ones. However, if you decide to not or only partially merge the tooling improvements in #39, you might have to adapt some PHP code, particularly use statements and/or class references in comments. This is also the reason why CI currently fails.

Copy link
Member

@johnbillion johnbillion left a comment

Choose a reason for hiding this comment

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

This all looks good. The Mine link handling might need reworking again when I get to #15.

@johnbillion johnbillion merged commit b8a8f9b into develop Jan 26, 2021
@johnbillion johnbillion deleted the php-review branch January 26, 2021 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants