add --topic arg to rss-bot script #747
Conversation
There was a problem hiding this comment.
@dimisjim Thanks for submitting this feature 👍
I expect existing users could benefit from this, but would also appreciate the option of retaining the existing behavior.
In addition, it would be useful if you could adjust the documentation to take into account any new option.
zulip/integrations/rss/rss-bot
Outdated
| "--topic", | ||
| dest="topic", | ||
| help="The Stream Topic to which to send RSS messages.", | ||
| default="stream events", |
There was a problem hiding this comment.
This changes the default behavior which currently exists; is there a reason for this?
There was a problem hiding this comment.
I've removed the default value via: 4de0952
Is my assumption correct that it will now fallback to data.feed.title or feed_url if no --topic arg is specified?
zulip/integrations/rss/rss-bot
Outdated
| @@ -241,7 +248,7 @@ for feed_url in feed_urls: | |||
| # entries in reverse chronological order. | |||
| break | |||
|
|
|||
| feed_name = data.feed.title or feed_url # type: str | |||
| feed_name = opts.topic | |||
There was a problem hiding this comment.
While this change sets the topic, this doesn't preserve the existing behavior, when a fixed topic is not specified.
There was a problem hiding this comment.
should be addressed via: 4de0952
zulip/integrations/rss/rss-bot
Outdated
| @@ -42,6 +42,10 @@ You can customize the location on the feed file and recipient stream, e.g.: | |||
|
|
|||
| /usr/local/share/zulip/integrations/rss/rss-bot --feed-file=/path/to/my-feeds --stream=my-rss-stream | |||
|
|
|||
| optionally, you can also specify the name of the stream topic, e.g.: | |||
|
|
|||
| /usr/local/share/zulip/integrations/rss/rss-bot --feed-file=/path/to/my-feeds --stream=my-rss-stream --topic=my-topic-name | |||
There was a problem hiding this comment.
Maybe specify an example topic containing spaces, e.g. --topic="topic name", since that's more typical naming style.
| @@ -248,7 +251,7 @@ for feed_url in feed_urls: | |||
| # entries in reverse chronological order. | |||
| break | |||
|
|
|||
| feed_name = opts.topic | |||
| feed_name = opts.topic or data.feed.title or feed_url # type: str | |||
There was a problem hiding this comment.
I would expect feed_url to often be too long; do we want to clamp too long topics here in the client code?
There was a problem hiding this comment.
not sure what your suggestion is here really.
data.feed.title or feed_url was already here before my PR
|
The feature has been implemented in #785. |
|
Heads up @dimisjim, we just merged some commits that conflict with the changes you made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
No description provided.