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

Add initial functionality and tests #1

Merged
merged 5 commits into from
May 11, 2022
Merged

Add initial functionality and tests #1

merged 5 commits into from
May 11, 2022

Conversation

dlh01
Copy link
Member

@dlh01 dlh01 commented May 7, 2022

Summary

As titled.

Notes for reviewers

None.

Changelog entries

Added

  • Initial release.

Changed

Deprecated

Removed

Fixed

Security

Copy link
Member

@kevinfodness kevinfodness left a comment

Choose a reason for hiding this comment

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

🍣

*/
private function run()
{
if (\is_array($this->path)) {
Copy link
Member

Choose a reason for hiding this comment

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

I generally prefer using use statements up top rather than fully qualified paths in the class body. Additionally, the PHP builtins typically don't require a use statement or a fully qualified path (unlike custom functions in WP or other libraries).

Copy link
Member Author

Choose a reason for hiding this comment

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

Built-in functions don't require qualification, true, but they can be made infinitesimally faster in some cases when they're fully qualified: PHP-CS-Fixer/PHP-CS-Fixer#3995.

It turns out I was using an old version of PHP-CS-Fixer that added namespaces to all built-in functions. Now that I've upgraded and only some functions are namespaced, I think I'll keep away from use statements for now, since that would make for an inconsistency: some functions would have use statements, some wouldn't.

@dlh01 dlh01 merged commit 1c29613 into main May 11, 2022
@dlh01 dlh01 deleted the 1.0 branch May 11, 2022 17:32
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.

None yet

2 participants