Skip to content

Create Minimal_Theme_Preparation#56

Merged
felixarntz merged 11 commits into
trunkfrom
feature/create-minimal-theme-preparation
Jan 25, 2023
Merged

Create Minimal_Theme_Preparation#56
felixarntz merged 11 commits into
trunkfrom
feature/create-minimal-theme-preparation

Conversation

@jjgrainger
Copy link
Copy Markdown
Contributor

@jjgrainger jjgrainger commented Jan 11, 2023

Adds the Minimal_Theme_Preparation class and wp-empty-theme theme.

  • Addresses acceptance criteria in Create Use_Minimal_Theme_Preparation  #39
  • While writing test the get_theme_root() method was failing. Looking into this the get_raw_theme_root() function calls the get_theme_roots() function which uses the theme_roots transient to search for a valid theme root based on the theme slug. Tests were failing because the theme_roots transient wasn't being updated when registering the new theme directory with register_theme_directory() in the prepare method. I've included a search_theme_directories( true ); function call after registering the theme directory in the prepare method in order to force update this transient.

Closes #39

@jjgrainger jjgrainger added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall plugin infrastructure Milestone 1 labels Jan 12, 2023
@jjgrainger jjgrainger marked this pull request as ready for review January 13, 2023 12:08
Copy link
Copy Markdown
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@jjgrainger Mostly looks good. Only one thing to fix in the code I think, but also we should add unit tests.

Comment thread includes/Checker/Preparations/Use_Minimal_Theme_Preparation.php
Comment thread includes/Checker/Preparations/Use_Minimal_Theme_Preparation.php
@felixarntz felixarntz mentioned this pull request Jan 13, 2023
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php Outdated
Comment thread includes/Checker/Preparations/Use_Minimal_Theme_Preparation.php
Copy link
Copy Markdown
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@jjgrainger Left some feedback on the test class.

Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php
@jjgrainger jjgrainger requested a review from felixarntz January 16, 2023 10:12
Copy link
Copy Markdown
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@jjgrainger Almost good to go now, left 3 small final comments.

Comment thread includes/Checker/Preparations/Use_Minimal_Theme_Preparation.php Outdated
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php Outdated
Copy link
Copy Markdown
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks @jjgrainger, Left few final feedback and questions.

Comment thread includes/Checker/Preparations/Use_Minimal_Theme_Preparation.php
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Use_Minimal_Theme_Preparation_Tests.php Outdated
Comment thread test-content/themes/wp-empty-theme/style.css
Copy link
Copy Markdown
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks @jjgrainger, Only one out standing review #56 (comment)

Copy link
Copy Markdown
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thanks @jjgrainger!

@felixarntz felixarntz merged commit 620f4d3 into trunk Jan 25, 2023
@felixarntz felixarntz deleted the feature/create-minimal-theme-preparation branch January 25, 2023 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Infrastructure Issues for the overall plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create Use_Minimal_Theme_Preparation

3 participants