Skip to content

Improved rss readme#12157

Merged
bholmesdev merged 10 commits intomainfrom
feat/improved-rss-readme
Oct 15, 2024
Merged

Improved rss readme#12157
bholmesdev merged 10 commits intomainfrom
feat/improved-rss-readme

Conversation

@bholmesdev
Copy link
Copy Markdown
Contributor

Changes

This tightens up the RSS README to remove unneeded guide pages (consistent with integration readmes) and reduce complexity of config options.

  • Remove the guide section and reference our docs entry
  • Inline the RSSFeedItem docs under items
  • Remove a duplicate docs section for content
  • Update types to show field types instead of object
  • Add support and contributing sections based on other integration READMEs

Testing

N/A

Docs

Follow up from this discussion on the RSS docs.

Follow ups

We should remove the inline documentation for item options and link to this README as a single source of truth. This should prevent out-of-date options. We should also clearly link to this README to learn more about config options.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Oct 8, 2024

🦋 Changeset detected

Latest commit: 46c1bf9

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

Co-authored-by: Armand Philippot <git@armand.philippot.eu>
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Hey @bholmesdev this is fantastic! (And, makes me really want to see this as a proper integration 😅 )

Left some stray thoughts below! 🙌

## `rssSchema`
Type: `Record<string, string> (optional)`

An object mapping a set of `xmlns` suffixes to strings of metadata on the opening `<rss>` tag.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So two things here:

  1. A second sentence like "This is used to/for" with plain language explanation of the purpose here would be nice.

  2. If the URL is just an example, then standard practice is to use example.com EVERYWHERE, even when other URLs seem fun.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This took me down a rabbithole to understand what xmlns even is. Learned a lot! Updated the entry to give a practical example

Copy link
Copy Markdown
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

I leave the final OK to Sarah :) once she gives the gree light, we can merge it

bholmesdev and others added 5 commits October 10, 2024 08:16
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
@sarah11918
Copy link
Copy Markdown
Member

Thanks @bholmesdev ! Left some follow up and I think you should have all the feedback you need from me to make your final judgement calls, and merge at will! 🫡

@bholmesdev bholmesdev merged commit 925cff3 into main Oct 15, 2024
@bholmesdev bholmesdev deleted the feat/improved-rss-readme branch October 15, 2024 15:13
@astrobot-houston astrobot-houston mentioned this pull request Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants