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

Remove unneeded framework files #56

Merged
merged 5 commits into from Apr 19, 2019

Conversation

@schlessera
Copy link
Member

commented Apr 18, 2019

As the tests package require several WP-CLI commands, and therefore also directly requires the WP-CLI framework, there's no point in having a (currently unsynced) copy of the three framework files utils.php, Process.php and ProcessRun.php included with the tests package.

This PR removes theses files, and makes the bootstrapping code more robust to find the version of the files that comes with the framework.

A side benefit is that there's only 1 single location now for these files, and all packages will always be up-to-date without additional PRs.

Depends on #53

Related #44

schlessera added some commits Apr 18, 2019

@schlessera

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

This is technically a breaking change, as the files that were initially included are now gone.

However, I doubt that any third-party package directly relied on them being in that specific location. As long as they are loaded and usable, everything should still work as expected.

@schlessera

This comment has been minimized.

Copy link
Member Author

commented Apr 19, 2019

Additional note: the PHP Unit tests were already using the utils.php from the framework dependency, instead of the one that was included with this package. This means that up until now, the helper functions used in PHP Unit were not the same as the ones used in the Behat tests. This could have caused (or actually did) hard to diagnose bugs.

@schlessera schlessera requested a review from wp-cli/committers Apr 19, 2019

@schlessera schlessera added this to the 2.0.14 milestone Apr 19, 2019

@jrfnl
Copy link
Contributor

left a comment

Just some small remarks to prevent new issues from being introduced.

features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved
features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved
features/bootstrap/FeatureContext.php Outdated Show resolved Hide resolved

@schlessera schlessera merged commit 96e005c into master Apr 19, 2019

2 checks passed

DEP All dependencies are resolved
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@delete-merged-branch delete-merged-branch bot deleted the remove-unneeded-framework-files branch Apr 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.