Adding RSS feed & sitemap #774
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Forgot to add the slugs for the different outputs:
Sitemaps do not show on |
There was a problem hiding this comment.
Pull Request Overview
This PR adds RSS feed and sitemap functionality to the Unitary Foundation website. The RSS feed combines both blog posts and grants into a single feed, while the sitemap provides better SEO and discoverability.
Key changes:
- Creates a combined RSS feed that includes both blog posts and grants, limited to the last 20 entries
- Adds sitemap generation with filtering to exclude test and image pages
- Updates the layout to include links to both the new RSS feed and sitemap
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/pages/rss.xml.ts | New RSS feed implementation combining blog posts and grants |
| src/layouts/Layout.astro | Adds RSS and sitemap links to the HTML head section |
| package.json | Adds @astrojs/sitemap dependency |
| astro.config.mjs | Configures sitemap integration with page filtering |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
src/pages/rss.xml.ts:21
- Variable name 'combined_items' uses snake_case which is inconsistent with JavaScript naming conventions. Consider using camelCase: 'combinedItems'.
const combined_items: RSSItem[] = []
cosenal
left a comment
cosenal
left a comment
There was a problem hiding this comment.
Imho we should keep the two feeds (blog posts and grants) separated, and the first one already exists.
astro.config.mjs
Outdated
| react(), | ||
| dotHtmlRedirects(), | ||
| sitemap({ | ||
| filter: (page) => page !== 'https://unitary.foundation/test/' && |
There was a problem hiding this comment.
Perhaps we can remove the test page all together.
There was a problem hiding this comment.
Removed locally. Will run some tests for this and other commits from suggestions ^and push updated version.
Individual slugs for RSS grant entries. Copilot recommendation. Co-authored-by: Copilot <[email protected]>
Use constant for `allowedHtmlTags`. Copilot recommendation. Co-authored-by: Copilot <[email protected]>
|
Yeah, leaving previously existing RSS in place seems wisest. At the very least, it won't break the feed of anyone already using it. |
|
@jbolns Once it's ready for a re-review, let me know 🙏 |
|
Oops. Yup, ready for re-review, I think. @cosenal |
|
@jbolns fyi, next time you can use the button to «Re-request review» when it's ready |
|
Sorry, sorry, I always forget about those buttons. Goldfish memory. 😄 |
|
:) |
|
What we discussed in the comments of this PR was to have two separate feeds, one for posts, the other for grants. Instead, what I see here now is:
What we want instead of 2) is simply something like 1), but for grants (possibly in |
|
Apologies, I misunderstood, probably due to cognitive dissonance as I thought the issue was still open after so long specifically because the schema operations required for a combined feed are tricky. I'll make the adjustments later today or tomorrow and resubmit. |
No worries. Before making the changes, let me know if the requirements are all clear now. |
|
I think they are. Basically, the same as blog, but with a different collection, i.e., grants, and ideally located in the grants folder rather than at the root. But... at the risk of coming across as slightly petulant, are you sure? You do not publish grants as frequently as needed for a fully independent grants feed. |
For now, yes.
This is how the feed from your PR looks like: If that's in a separate grant feed, at least it's clear what it is about. |
|
Fair enough. Will try and push tomorrow. |
|
Done, I think. |
2 participants
PR adds:
The first commit is unlikely ready for merging. There is an existing RSS for blogs only. It is necessary to decide first if to keep it like that (2 RSS feeds, one for blogs, one combining blogs and grants) or use only the combined version.
Closes #49
@cosenal . Adding sitemap here since you need it anyway and the two things typically go hand in hand.