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

Added a WP CLI command to rebuild the sitemap. #7930

Merged
merged 4 commits into from
Nov 13, 2017
Merged

Conversation

zinigor
Copy link
Member

@zinigor zinigor commented Oct 4, 2017

This will help in testing and will allow users at least a low level way to rebuild the sitemap.

Changes proposed in this Pull Request:

  • Added a CLI command to rebuild the sitemap.

Proposed changelog entry for your changes:

  • Added a CLI command to rebuild the sitemap.

This will help in testing and will allow users at least a low level way to rebuild the sitemap.
@zinigor zinigor added [Feature] Sitemaps [Status] Needs Review This PR is ready for review. [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it labels Oct 4, 2017
@zinigor zinigor requested a review from a team as a code owner October 4, 2017 12:05
Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

I get Fatal error: Class 'Jetpack_Sitemap_Builder' not found even when the sitemaps module is enabled.

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Review This PR is ready for review. labels Oct 24, 2017
* @synopsis <rebuild>
*/
public function sitemap( $args, $assoc_args ) {
if ( ! Jetpack::is_active() ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we check if sitemaps is active too?

WP_CLI::error( __( 'Jetpack is not currently connected to WordPress.com', 'jetpack' ) );
}
if ( ! Jetpack::is_module_active( 'sitemaps' ) ) {
WP_CLI::error( __( 'Jetpack Sitemaps module is not currently active. Activate it if first if you want to work with sitemaps.', 'jetpack' ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a typo here: Activate it if first if. Two ifs.

@oskosk
Copy link
Contributor

oskosk commented Nov 10, 2017

I'm still getting the error when sitemaps is active and trying to use the cli.

By var_dumping what is returned by get_included_files() by the time the cli command tries to rebuild sitemaps I get to see that neither jetpack/modules/sitemaps/sitemaps.php nor jetpack/modules/sitemaps/sitemap-builder.php are included at that point.
Although jetpack/modules/sitemap.php is included but I don’t understand what is happening. Maybe we should just require the site-builder file at that point.

Copy link
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM ! Added a commit removing an extra if in the text.

@oskosk oskosk added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Nov 10, 2017
@oskosk oskosk merged commit 1439147 into master Nov 13, 2017
@oskosk oskosk deleted the add/sitemap-cli-rebuild branch November 13, 2017 13:56
jeherve added a commit that referenced this pull request Nov 24, 2017
oskosk pushed a commit that referenced this pull request Nov 28, 2017
* Changelog 5.6: create base for changelog.

* Update changelog with 5.5.1 info.

* Changelog: add #7930 and #8238

* Changelog: add #8076

* Changelog: add #8100

* Changelog: add #8117

* Changelog: add #8141

* Changelog: add #8143

* Changelog: add #8147

* Changelog: add #8149

* Changelog: add #8153

* Changelog: add #8173

* Changelog: add #8184

* Changelog: add #8196

* Changelog: add #8199

* Changelog: add #8093

* Changelog: add #8171

* Changelog: add #8182

* Changelog: add #8202, #8222

* Changelog: add #8228

* Changelog: add #8240

* Changelog: add #8251

* remove AL card change
@kraftbj kraftbj removed the [Status] Ready to Merge Go ahead, you can push that green button! label Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Sitemaps [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants