-
-
Notifications
You must be signed in to change notification settings - Fork 1.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: Docs say pubDate is optional, when it's actually required. #9575
Comments
Thanks for letting us know @JaneSmith ! I'm looking through the package now and seeing that the field does appear to be marked as optional in the RSS package code: So I'm not entirely sure what's up! This sounds like it needs confirming with our devs so we know what the proper thing to document is. I'll ask @bholmesdev to investigate and let us know the best course of action, what the official word is, and then once we know that, if you'd like to help with a PR to fix the docs/error message as necessary, we'd love to have you contribute! 🙌 |
If I'm right, this is a bug and not a documentation issue (except an outdated README for From the changelog, And I can reproduce the bug: here is a minimal reproduction (just delete one of the |
Thank you @ArmandPhilippot ! I wonder whether we should do what we do with the other Astro integrations and only have the README there be a link to docs? That would avoid a problem like this in future, when devs probably aren't thinking of updating that README when they make a change. |
Ah yes that could be a good idea since all other packages (at least the ones I checked) use the same format. And I don't think anyone would try to use it outside of Astro... so it makes sense to link to the documentation. Should the change be made in the current PR or is a new PR better? I'm asking this in relation to the changelog... if we make the change now, the changeset specifying that the previous README was wrong no longer makes sense... but it could be useful for users. |
Thanks for that fix @ArmandPhilippot! I see that PR is merged now, so a new PR makes sense to me. We can also safely close this documentation issue now that docs.astro.build matches expectations. |
@ArmandPhilippot update: I just PR'd a docs change! I decided config options should be left here, since there are a number of options our docs guide does not document. I'll make a docs PR as well to directly link to the README for config reference. |
📚 Subject area/topic
RSS
📋 Page(s) affected (or suggested, for new content)
https://docs.astro.build/en/guides/rss/
📋 Description of content that is out-of-date or incorrect
The docs say the following:
Note the "optional" at the end. But actually, pubDate appears to be required. When leaving it out in an
rss.xml.js
file that callsrss(...)
, I get the following error in the Astro dev server:The @astrojs/rss NPM page on the other hand does correctly state that pubDate is required:
So the documentation is not consistent across the board.
Additional information
I also noticed that one of the error messages that Astro RSS can produce implies that pubDate is not required:
Note the "if provided", implying they can be left out, when actually both are required. I'm not sure if I should open a separate issue for that or not. It's technically unrelated to documentation.
The text was updated successfully, but these errors were encountered: