-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Sitemap should only include page
routes
#7656
Conversation
🦋 Changeset detectedLatest commit: 44417b4 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Flagging this for @withastro/maintainers-docs just in case there are concerns! |
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.
Thanks for checking in with docs @natemoo-re !
Reminder: this PR does not update the README (which may be fine!)
Here is how the README introduces the sitemap
integration. Are you still comfortable with this description? (I'll point out that it does NOT strictly say page routes in the very first line, even though that may be the expectation.)
I'm happy to sign off and approve here, and will leave it to you to confirm that you still believe the README intro presents this integration accurately. If not, then update as you see fit! 🙌
81a305f
to
791e969
Compare
Changes
This is a MAJOR (breaking) change that will bump
@astrojs/sitemap
to 2.0.0!@astrojs/sitemap
only includes page routessitemap
as generating a sitemap from your pages, but all routes were erroneously included. This behavior did not match user expectations and non-page routes needed to be filtered out manually.Testing
Our tests only covered pages, I added an endpoint and redirect to the suite.
Docs
This change aligns with our existing docs for this package