Skip to content

Add Force_Single_Plugin_Preparation class#68

Merged
vishalkakadiya merged 29 commits into
trunkfrom
feature/add-force-single-plugin-preparation
Feb 7, 2023
Merged

Add Force_Single_Plugin_Preparation class#68
vishalkakadiya merged 29 commits into
trunkfrom
feature/add-force-single-plugin-preparation

Conversation

@vishalkakadiya
Copy link
Copy Markdown

Adds the Force_Single_Plugin_Preparation class

Closes #58

Copy link
Copy Markdown
Contributor

@jjgrainger jjgrainger left a comment

Choose a reason for hiding this comment

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

Thanks @vishalkakadiya overall looking good, but left some minor feedback.

Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php Outdated
Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php
Comment thread tests/Checker/Preparations/Force_Single_Plugin_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Force_Single_Plugin_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Force_Single_Plugin_Preparation_Tests.php Outdated
Vishal Kakadiya and others added 6 commits February 1, 2023 10:32
Co-authored-by: Joe Grainger <904708+jjgrainger@users.noreply.github.com>
Co-authored-by: Joe Grainger <904708+jjgrainger@users.noreply.github.com>
Co-authored-by: Joe Grainger <904708+jjgrainger@users.noreply.github.com>
Co-authored-by: Joe Grainger <904708+jjgrainger@users.noreply.github.com>
@vishalkakadiya
Copy link
Copy Markdown
Author

Thank you! @jjgrainger

@felixarntz It is ready for your review now. Thank you!

@vishalkakadiya
Copy link
Copy Markdown
Author

@jjgrainger I have addressed your feedback, can you re-review it again. Thank you! 🙂

Copy link
Copy Markdown
Contributor

@jjgrainger jjgrainger left a comment

Choose a reason for hiding this comment

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

Thanks @vishalkakadiya approved!

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.

@vishalkakadiya This is going in the right direction. There are a few quirks with the production logic that need to be ironed out, but otherwise just suggestions to enhance the tests and a few nit-picks.

Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php Outdated
Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php
Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php Outdated
Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php Outdated
Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php Outdated
Comment thread tests/Checker/Preparations/Force_Single_Plugin_Preparation_Tests.php Outdated
Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php
Comment thread tests/Checker/Preparations/Force_Single_Plugin_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Force_Single_Plugin_Preparation_Tests.php Outdated
Comment thread tests/Checker/Preparations/Force_Single_Plugin_Preparation_Tests.php Outdated
Vishal Kakadiya and others added 8 commits February 2, 2023 11:54
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@vishalkakadiya
Copy link
Copy Markdown
Author

@felixarntz I have addressed your feedback and left one comment, you can do the second round of review now. Thank you!

@vishalkakadiya vishalkakadiya requested review from jjgrainger and removed request for jjgrainger February 2, 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.

@vishalkakadiya Almost good to go. One follow up point from my previous review that wasn't fully addressed, and two very small nit-picks - they wouldn't be blocking, but since the one test feedback needs to be addressed anyway, it would be great if you could address those two small things as well.

Comment thread includes/Checker/Preparations/Force_Single_Plugin_Preparation.php Outdated
Comment thread plugin-check.php
Comment thread tests/Checker/Preparations/Force_Single_Plugin_Preparation_Tests.php Outdated
@jjgrainger jjgrainger requested a review from felixarntz February 6, 2023 13:29
@vishalkakadiya
Copy link
Copy Markdown
Author

@felixarntz I have addressed the remaining feedback on it. It is ready for review now. Thank you!

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.

@vishalkakadiya Pretty much LGTM, just 3 nit-picks that would be good to address. Feel free to merge afterwards without another review.

Comment thread tests/Checker/Check_Context_Tests.php Outdated
Comment thread tests/Plugin_Context_Tests.php Outdated
Comment thread tests/Plugin_Main_Tests.php Outdated
Vishal Kakadiya and others added 3 commits February 7, 2023 10:43
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
Co-authored-by: Felix Arntz <felixarntz@users.noreply.github.com>
@vishalkakadiya
Copy link
Copy Markdown
Author

@felixarntz Thank you for the feedback, I have fixed those and merging this now.

@vishalkakadiya vishalkakadiya merged commit 776284b into trunk Feb 7, 2023
@vishalkakadiya vishalkakadiya deleted the feature/add-force-single-plugin-preparation branch February 7, 2023 05:19
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.

Create Force_Single_Plugin_Preparation

3 participants