Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

integration-docs: Update the RSS integration doc.#872

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Niloth-p wants to merge1 commit intozulip:main
base:main
Choose a base branch
Loading
fromNiloth-p:docs/rss

Conversation

Niloth-p
Copy link
Contributor

@Niloth-pNiloth-p commentedJun 5, 2025•
edited
Loading

Screenshots

RSS doc link
image
image

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (seecommit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

Copy link

@laurynmmlaurynmm left a comment

Choose a reason for hiding this comment

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

@Niloth-p - Thanks again for this integration doc update! Let me know if you have any questions about my review comments! Some here are reflections on other pending integration doc changes that are similar.

Niloth-p reacted with rocket emoji
location is `~/.cache/zulip-rss`.

- `--stream`: The name of the Zulip channel you want to receive
notifications in. By default, messages are sent to the `rss` channel.

Choose a reason for hiding this comment

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

We should be consistent in using quotes or backticks for channel names across all integration docs. Here we use backticks, but in the SVN or IRC updates we used quotes.

Niloth-p reacted with thumbs up emoji
@Niloth-p
Copy link
ContributorAuthor

Thank you for the review! Updated as recommended.
I've switched to using backticks instead of double quotes for channel and topic names, and added the prefix# in front of channel names (because that's the convention in the Help docs -example).

Screenshots

image
image


!!! tip ""

Note that [the Zapier integration][1] is usually a simpler way to
[The Zapier integration][1] is usually a simpler way to
Copy link
Contributor

@alyaalyaJul 1, 2025•
edited
Loading

Choose a reason for hiding this comment

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

Is this only if you're OK with all the messages going to the same topic? If so, it should be something like:

Suggested change
[The Zapier integration][1] is usually a simpler way to
For sending messages to a single topic, [the Zapier integration][1] is a simpler way to

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Not particularly, Zapier can send DMs or construct the topic name from the payload too.

I think the direct integration may be preferred by folks who want to customize the integration, or use the--unwrap or--math options or wish to send DMs without sharing their credentials with Zapier.

`--feed-file` [option](#configuration-options) to the integration
script.

1. You can run the bot to send summaries of RSS entries from your favorite
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1.You can run the bot to send summaries of RSS entries from your favorite
1.Run the bot to send summaries of RSS entries from your favorite

Niloth-p reacted with thumbs up emoji

`{{ integration_path }}/rss-bot`

1. Optionally, pass command-line arguments to re-configure the integration.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1.Optionally, passcommand-line arguments to re-configure the integration.
1.Passcommand-line arguments to re-configure the integration. See[the configuration options](#configuration-options below.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

The reason for having the "Optionally" word, and mention the "list of options" is that there are some integrations that require all of their supported command-line arguments. We're trying to say that these command-line arguments are not necessary, but if the user wants to configure it further, they can. And the full list of options is available below, but the user can choose to use some of those options without using the rest.

I've retained the "Optionally" word, but rephrased the link sentence as suggested.

Comment on lines 45 to 46
See [the configuration options](#configuration-options) for the list of
options.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
See [the configuration options](#configuration-options) for the list of
options.

1. Configure a crontab entry to keep the integration running.

This sample crontab entry processes feeds stored in the default
location and posts to the `rss` topic in the `#news` channel every 5
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
location and posts to the `rss` topic in the`#news` channel every 5
location and posts to the `rss` topic in the**#news** channel every 5

location is `~/.cache/zulip-rss`.

- `--stream`: The name of the Zulip channel you want to receive
notifications in. By default, messages are sent to the `#rss` channel.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
notifications in. By default, messages are sent to the`#rss` channel.
notifications in. By default, messages are sent to the**#rss** channel.

Copy link
ContributorAuthor

@Niloth-pNiloth-p left a comment

Choose a reason for hiding this comment

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

Thank you for the review!

Screenshots

image
image


!!! tip ""

Note that [the Zapier integration][1] is usually a simpler way to
[The Zapier integration][1] is usually a simpler way to
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Not particularly, Zapier can send DMs or construct the topic name from the payload too.

I think the direct integration may be preferred by folks who want to customize the integration, or use the--unwrap or--math options or wish to send DMs without sharing their credentials with Zapier.


`{{ integration_path }}/rss-bot`

1. Optionally, pass command-line arguments to re-configure the integration.
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

The reason for having the "Optionally" word, and mention the "list of options" is that there are some integrations that require all of their supported command-line arguments. We're trying to say that these command-line arguments are not necessary, but if the user wants to configure it further, they can. And the full list of options is available below, but the user can choose to use some of those options without using the rest.

I've retained the "Optionally" word, but rephrased the link sentence as suggested.

@alya
Copy link
Contributor

alya commentedJul 2, 2025

OK, I'm happy for other reviewers (@timabbott ?) to take the next look.

Niloth-p reacted with hooray emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@alyaalyaalya left review comments

@laurynmmlaurynmmlaurynmm left review comments

Assignees
No one assigned
Labels
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@Niloth-p@alya@laurynmm@zulipbot
[8]ページ先頭
©2009-2025 Movatter.jp