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

Add title/description for mu-plugins. #305

Merged
merged 9 commits into from
May 25, 2022
Merged

Add title/description for mu-plugins. #305

merged 9 commits into from
May 25, 2022

Conversation

alexstine
Copy link
Contributor

@alexstine alexstine commented Nov 18, 2021

Currently, only name is populated for mu-plugins. This PR will also fill in the title and add the description.

@alexstine alexstine requested a review from a team as a code owner November 18, 2021 21:07
@schlessera
Copy link
Member

Thanks for the PR, @alexstine !

The code looks good at first glance, but to merge it, we'd need a Behat test as well to ensure it actually does (and keeps doing) what it is supposed to do. Do you care to add such a test?

@alexstine
Copy link
Contributor Author

@schlessera Can you find another contributor to implement the test? I just don't have time at the moment and I'm sure you are probably in the same boat. Maybe one for next CLI meeting?

Thanks.

@wojsmol
Copy link
Contributor

wojsmol commented May 12, 2022

@alexstine Changes reverted in 10b75d3 was correct.

@alexstine
Copy link
Contributor Author

@wojsmol Better now? Just refreshed the fork/branch.

@wojsmol
Copy link
Contributor

wojsmol commented May 12, 2022

@alexstine LGTM now. Tests are requiring @schlessera approval to run.

@alexstine
Copy link
Contributor Author

@wojsmol These check fails don't look related to my changes. Maybe worth a re-run?

@swissspidy
Copy link
Member

Should we also add support for descriptions?

Tests are all passing now btw

@alexstine
Copy link
Contributor Author

Nothing like all green checks. 👍

If you think it is a good idea, I can try to get descriptions working or can handle in another PR.

@swissspidy
Copy link
Member

If you're up for it, that would be great. Can be in this PR or a new one, whatever works best for you. Code-wise it should be mostly copy & paste

@alexstine alexstine changed the title Add title for mu-plugins. Add title/description for mu-plugins. May 25, 2022
@alexstine
Copy link
Contributor Author

@swissspidy Done. Not sure if the tests will pass but hopefully so. Gave this a test on my test server, seems solid enough.

features/plugin.feature Outdated Show resolved Hide resolved
@swissspidy swissspidy added this to the 2.1.5 milestone May 25, 2022
@swissspidy swissspidy merged commit 68f1fcf into wp-cli:master May 25, 2022
@alexstine alexstine deleted the add/mu-plugin-title branch May 27, 2022 14:02
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.

4 participants