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 the step "importThemeStarterContent" #1994

Open
bph opened this issue Nov 12, 2024 · 8 comments
Open

Remove the step "importThemeStarterContent" #1994

bph opened this issue Nov 12, 2024 · 8 comments
Assignees
Labels
[Type] Documentation Improvements or additions to documentation

Comments

@bph
Copy link
Collaborator

bph commented Nov 12, 2024

The documentation lists a step: "importThemeStarterContent"
Implementation issue shows, that it's an option on the InstallTheme step.

It's already included in the installTheme step. documentation

Example quoted from the PR:

{
  "steps": [
    {
      "step":"installTheme",
      "themeZipFile":{
        "resource":"wordpress.org/themes",
        "slug":"twentytwenty"
       },
       "options":{
         "importStarterContent": true
       }
    }
  ]
}
@bph bph self-assigned this Nov 12, 2024
@bph bph added [Type] Documentation Improvements or additions to documentation [Type] Developer Documentation Documentation for developers labels Nov 12, 2024
@bph
Copy link
Collaborator Author

bph commented Nov 12, 2024

There are two files:

  • packages/playground/blueprints/src/lib/steps/import-theme-starter-content.ts
  • packages/playground/blueprints/src/lib/steps/import-theme-starter-content.spec.ts

I can't figure out where to make the updates to remove the step from the documentation, though.

@bph
Copy link
Collaborator Author

bph commented Nov 12, 2024

And maybe it shouldn't be under steps at all but added to the InstallTheme step..

I see in import-theme-starter-content.ts

/**
 * @inheritDoc importThemeStarterContent
 * @example
 *
 * <code>
 * {
 * 		"step": "importThemeStarterContent"
 * }
 * </code>
 */

and further down

	step: 'importThemeStarterContent';
	/**
	 * The name of the theme to import content from.
	 */
	themeSlug?: string;
}

/**
 * Imports a theme Starter Content into WordPress.
 *
 * @param playground Playground client.
 */

Should that be all that needs to be deleted?

@bph bph removed the [Type] Developer Documentation Documentation for developers label Nov 12, 2024
@bgrgicak
Copy link
Collaborator

@bph importThemeStarterContent is both a step and an option in the installTheme step. Why would you like to remove the step?

It's nice to have both in case you need to do something more custom. For example, you can install a theme that doesn't have starter content, add it using a writeFile step, and run importThemeStarterContent later.

@bgrgicak
Copy link
Collaborator

There are two files:

* packages/playground/blueprints/src/lib/steps/import-theme-starter-content.ts

* packages/playground/blueprints/src/lib/steps/import-theme-starter-content.spec.ts

I can't figure out where to make the updates to remove the step from the documentation, though.

You would update the import-theme-starter-content.ts file to make documentation example changes. The /import-theme-starter-content.spec.ts file only contains unit tests that automatically check if the step works.

@bgrgicak
Copy link
Collaborator

To answer your question about deleting steps.

Check if the step is used

We collect step usage stats in Google Analytics, so before we remove anything, we can check how frequently it is used. The importThemeStarterContent step is rarely used.

Before we can remove it, we usually mark it as deprecated for a while before actually removing it.

Deleting a step

To delete a step you need to delete the step interface, in this case, it would be this code:

/**
* @inheritDoc importThemeStarterContent
* @example
*
* <code>
* {
* "step": "importThemeStarterContent"
* }
* </code>
*/
export interface ImportThemeStarterContentStep {
/** The step identifier. */
step: 'importThemeStarterContent';
/**
* The name of the theme to import content from.
*/
themeSlug?: string;
}

You also need to delete any references to the interface by searching the Playground project for its name (i.e. ImportThemeStarterContentStep).

Finally, you need to remove the step from the blueprint schema, in this case, it would be this code:

{
"type": "object",
"additionalProperties": false,
"properties": {
"progress": {
"type": "object",
"properties": {
"weight": {
"type": "number"
},
"caption": {
"type": "string"
}
},
"additionalProperties": false
},
"step": {
"type": "string",
"const": "importThemeStarterContent",
"description": "The step identifier."
},
"themeSlug": {
"type": "string",
"description": "The name of the theme to import content from."
}
},
"required": ["step"]
},

@bgrgicak bgrgicak moved this from Inbox to Needs Author's Reply in Playground Board Nov 13, 2024
@bph
Copy link
Collaborator Author

bph commented Nov 13, 2024

Thank you, @bgrgicak
Ah sorry, this wasn't clear. For this issue, I was only thinking of removing the step from the Documentation, not so much from Playground.

This PR #1521 was the only one I found about this, and it only mentions the installTheme option, unless I am missing something. There is also no example on how to use the step variation. My conclusion was that the documentation wasn't updated, and we can take care of it by removing it from the Documentation. Only when I tried to find the spot where the documentation originated, I found the step files, and was confused.

Before we can remove it, we usually mark it as deprecated for a while before actually removing it.

This was a misunderstanding, and I would love to update the documentation with a working example for the step variation on how to do this in a blueprint. The current version doesn't help here much. Or I don't understand it.

For example, you can install a theme that doesn't have starter content, add it using a writeFile step, and run importThemeStarterContent later.

That's a great use case. How would this be accomplished via blueprint?

@adamziel adamziel moved this from Needs Author's Reply to Inbox in Playground Board Nov 13, 2024
@bgrgicak
Copy link
Collaborator

The current version doesn't help here much. Or I don't understand it.

I agree, some of our Blueprint examples are so minimal that they don't do anything on their own or even break the site.

@bgrgicak
Copy link
Collaborator

That's a great use case. How would this be accomplished via blueprint?

Here is a minimal example which adds a new homepage section.

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Documentation Improvements or additions to documentation
Projects
Status: Inbox
Development

No branches or pull requests

3 participants