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.

Sign up for GitHub

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

Jump to bottom

feat(gatsby-plugin-sitemap): create sitemap index #12239

Merged
merged 6 commits into from
Mar 11, 2019
Merged

feat(gatsby-plugin-sitemap): create sitemap index #12239

merged 6 commits into from
Mar 11, 2019

Conversation

tomoyukikashiro
Copy link
Contributor

@tomoyukikashiro tomoyukikashiro commented Mar 2, 2019 •
edited
Loading

Description

Now gatsby-plugin-sitemap can not create sitemap index.
This PR allow us to create it by passing additional option to that plugin.

Also this plugin did not have any tests so I added them.

Related Issues

#12100

Details

// In your gatsby-config.js
siteMetadata: {
  siteUrl: `https://www.example.com`,
},
plugins: [
  {
    resolve: `gatsby-plugin-sitemap`,
    options: {
      sitemapSize: 5000
    }
  }
]

Above is the minimal configuration to split large sitemap.
When number of URL in sitemap is more than 5000 plugin will create sitemap (e.g. sitemap-0.xml, sitemap-1.xml) and index (e.g. sitemap.xml) files.

@tomoyukikashiro tomoyukikashiro self-assigned this Mar 3, 2019
@tomoyukikashiro
Copy link
Contributor Author

@pieh I assigned you as reviewer 🙇 because github suggested that you reviewed these file recently... I did not know what to do after making PR ( assigned someone as reviewer by myself or just wait).

wardpeet
wardpeet suggested changes Mar 4, 2019
View reviewed changes
Copy link
Contributor

@wardpeet wardpeet left a comment

Choose a reason for hiding this comment

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

Added a few comments. This looks pretty great. Big projects will benefit from this!

packages/gatsby-plugin-sitemap/src/gatsby-node.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-sitemap/src/gatsby-node.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-sitemap/src/gatsby-node.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-sitemap/src/gatsby-node.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-sitemap/src/gatsby-node.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-sitemap/src/gatsby-node.js Outdated Show resolved Hide resolved
packages/gatsby-plugin-sitemap/src/gatsby-node.js Outdated Show resolved Hide resolved
@wardpeet
Copy link
Contributor

wardpeet commented Mar 4, 2019

I also tested the plugin and works!

@wardpeet wardpeet added the status: awaiting author response Additional information has been requested from the author label Mar 4, 2019
@tomoyukikashiro
Copy link
Contributor Author

@wardpeet Thanks for your review 👍 I have one question !
↓
#12239 (comment)

@tomoyukikashiro tomoyukikashiro added status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response and removed status: awaiting author response Additional information has been requested from the author labels Mar 5, 2019
@wardpeet wardpeet added status: awaiting author response Additional information has been requested from the author and removed status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response labels Mar 7, 2019
@tomoyukikashiro tomoyukikashiro added status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response and removed status: awaiting author response Additional information has been requested from the author labels Mar 8, 2019
@wardpeet
Copy link
Contributor

Works like a charm, thank you!

@wardpeet wardpeet merged commit 55baf48 into gatsbyjs:master Mar 11, 2019
@tomoyukikashiro tomoyukikashiro deleted the topics/gatsby-plugin-sitemap-options branch March 11, 2019 22:34
@sidharthachatterjee
Copy link
Contributor

Published in [email protected]

muescha
muescha reviewed Mar 12, 2019
View reviewed changes
packages/gatsby-plugin-sitemap/src/gatsby-node.js
},
} = queryRecords
return new Promise(resolve => {
// sitemapv-index.xml is default file name. (https://git.io/fhNgG)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// sitemapv-index.xml is default file name. (https://git.io/fhNgG)
// sitemap-index.xml is default file name. (https://git.io/fhNgG)

sitemapv-index -> sitemap-index?

Copy link
Contributor

@wardpeet wardpeet Mar 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@muescha it's already merged, could you add a new pr? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

ok done -> #12512

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@muescha @wardpeet Thanks for that 😓

@muescha muescha mentioned this pull request Mar 12, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wardpeet wardpeet wardpeet approved these changes

@muescha muescha muescha left review comments

@pieh pieh Awaiting requested review from pieh

Assignees

@tomoyukikashiro tomoyukikashiro

Labels
status: awaiting reviewer response A pull request that is currently awaiting a reviewer's response
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tomoyukikashiro @wardpeet @sidharthachatterjee @muescha