-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[RSS] Get ready for content collections #5851
Conversation
🦋 Changeset detectedLatest commit: a3cd5e9 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 |
Updating to keep |
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.
Seems like a great refactor! Had a few comments around messaging and maybe found a typo?
This reverts commit 8530507.
1192595
to
c739709
Compare
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.
Quick Docs README review:
I've reviewed the accompanying PR to docs, which I know contains some of the same examples and sections. I'll let you look over those comments, and decide which ones are applicable here. It's OK if this README is more technical than the docs RSS page, which should be more oriented to practical usage.
But, I want to give you a chance to look over that review and
a) see if there's any suggestion there that you'd choose to carry into here.
b) decide whether there's anything you're happy to leave exclusively to docs, and cut from the README. (We favour as little duplication as possible, and keeping the READMES to a minimum, where it makes sense. This document won't often have to stand on its own since readers are far more likely to encounter the docs page, and search only works in docs proper, etc.)
Then, I'll take another last look over this, but I don't see anything that's going to hold this up!
@sarah11918 Made some tweaks based on docs feedback! Seems good-to-go from my end 👍 |
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.
Just re-add that missing "we" and good to go from Docs, @bholmesdev!
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Changes
rssSchema
to type-check content collections for RSS feedsimport.meta.glob
handling and move to apagesGlobToRssItems()
helper. This has a few benefits:items
array to accept a single type. Better for JS docs and docs.astro.build.src/pages/
entries" requirement more explicit with the function name.import.meta.glob
handling was implicit, this felt like the more encouraged solution.README changes
rssSchema
andpagesGlobToRssItems()
referenceimport.meta.env.SITE
recommendation to newcontext.site
Testing
pagesGlobToRssItems()
testsDocs