Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Move the new pattern logic into another file and unit test it #59

Merged
merged 4 commits into from
Feb 18, 2023

Conversation

kienstra
Copy link
Contributor

@kienstra kienstra commented Feb 16, 2023

Moves the logic to get a new pattern into a pure function, and unit tests it.

No intended change to how this works.

How to test

Not needed

@kienstra kienstra marked this pull request as ready for review February 16, 2023 19:27
Base automatically changed from saving-pattern-data-phil to update/saving-pattern-data February 16, 2023 19:37
Base automatically changed from update/saving-pattern-data to main February 16, 2023 19:38
@@ -131,7 +131,7 @@ function format_pattern_data( $pattern_data, $file ) {
*
* @return array
*/
function get_theme_patterns() {
function get_theme_patterns(): array {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Forgot to do this before

'slug' => $new_name,
'title' => $new_title,
)
);
Copy link
Contributor Author

@kienstra kienstra Feb 16, 2023

Choose a reason for hiding this comment

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

This is copy-pasted into get_new_pattern() so we can unit test it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No intended change to how it works.

Copy link
Contributor

@mike-day mike-day left a comment

Choose a reason for hiding this comment

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

This looks good, @kienstra! Great idea to add these unit tests.

@kienstra
Copy link
Contributor Author

Thanks so much, @mike-day!

@kienstra kienstra merged commit 75373ad into main Feb 18, 2023
@kienstra kienstra deleted the update/new-pattern-unit-test branch February 18, 2023 04:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants