Skip to content

Factor directory iterator as a trait#54

Merged
schlessera merged 4 commits intowp-cli:masterfrom
drzraf:factor-dir-iterator
Jul 31, 2018
Merged

Factor directory iterator as a trait#54
schlessera merged 4 commits intowp-cli:masterfrom
drzraf:factor-dir-iterator

Conversation

@drzraf
Copy link
Copy Markdown

@drzraf drzraf commented Jul 24, 2018

Factor directory iterator as a trait so that other extractor can rely on it and make extension generic so that test (<extension> === 'php') can be overridden if needed.

… on it.

and make extension generic so that test (<extension> === 'php') can be overriden if needed.
@drzraf drzraf changed the title Factor directory iterator as a trait so that other extractor can rely… Factor directory iterator as a trait Jul 24, 2018
@swissspidy
Copy link
Copy Markdown
Member

That would need the same change for the JS extractor to make things DRY.

private static $dir = '';

/**
* {@inheritdoc}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd use the docs from the ExtractorInterface here as there's nothing to inherit from otherwise.

Plus, we could describe the possible options a bit better. Perhaps the way WordPress core does it: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#1-1-parameters-that-are-arrays

*
* @param string $dir Root path to start the recursive traversal in.
* @param Translations $translations The translations instance to append the new translations.
* @param array $options
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As noted above, we could describe the possible options a bit better. Perhaps the way WordPress core does it: https://make.wordpress.org/core/handbook/best-practices/inline-documentation-standards/php/#1-1-parameters-that-are-arrays

@swissspidy swissspidy requested a review from schlessera July 28, 2018 12:08
@schlessera schlessera added the command:i18n-make-pot Related to 'i18n make-pot' command label Jul 31, 2018
@schlessera schlessera added this to the 0.1.0 milestone Jul 31, 2018
@schlessera schlessera merged commit f9e94e8 into wp-cli:master Jul 31, 2018
@schlessera
Copy link
Copy Markdown
Member

Thanks for the pull-request, @drzraf !

schlessera added a commit that referenced this pull request Jan 6, 2022
Factor directory iterator as a trait
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

command:i18n-make-pot Related to 'i18n make-pot' command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants