Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 DOMParser from Blueprints: installPlugin and installTheme #427
Remove DOMParser from Blueprints: installPlugin and installTheme #427
Changes from 35 commits
815acec
61e33f3
7eb0e26
304e256
9c34cf4
89a627d
d53a55a
98cde65
bfaef98
3f8eb82
f2ed29b
7840907
daf74b3
82ad36e
3520de3
579c568
534d10f
0615596
11bee55
6080b0d
fa4f559
3fce74e
2b8d31b
e5ae9b5
3dbd17a
ca7b8c1
9eed478
56cb9d3
28f7163
dc05730
81bb68f
a368526
7d4f233
30d5c9a
9ed3d75
3b67796
c121866
661eda9
3cbea6a
4487c3a
6b0f60f
1671b88
3526f57
6f81a34
977230f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I initially had this in mind as an internal utility to aid other steps, I wonder whether this makes sense as its own step 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The term "asset" seems ambiguous as part of a public API. Internally it means:
Technically it's generic, not limited to plugins and themes - but I can't imagine what else would be considered "assets" that has the above characteristics.
There could be an
installMuPlugin
blueprint step, same as for plugins but with targetmu-plugins
.Media files could be another kind of asset. Maybe an
installMedia
step that usesinstallAssets
. Unlike plugins/themes, it would not require a single container folder. And the step will need to run additional PHP to upload the files into the media library.So I think
installAsset
by itself may not be useful to expose as a blueprint step, and can probably stay an internal helper function.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry I got confused. It is in the steps directory and kind of looks like a step declaration, but it actually is an internal helper. My bad!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to put each step in a separate file and move helpers somewhere distinct, maybe a subdirectory? Let's figure this out separately