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
generic_rss_renderer refactor #2686
generic_rss_renderer refactor #2686
Conversation
Split out separate generic_rss_feed and utils.rss_writer from generic_rss_renderer. This allows generic_rss_feed and utils.rss_writer to be used by plugins to generate and write out rss feeds, but to modify them between initial generation and writing.
Using kw.pop to get a `creator` kwarg from ExtendedItem.__init__ makes `creator` required, but later code treats it as potentially False or None. Allow it to actually be optional (with default value None) to make ExtendedItem easier to subclass.
I'll add some docstrings later. Let me know if this kind of internal change justifies an entry in |
Sure, you can put in CHANGES something like:
Also, remember to add yourself to authors if you are not there. |
OK. I've added docstrings and updated CHANGES. (I'm already in AUTHORS from a previous merge.) As an aside, I'd love for someone to look over the related plugin work I'm doing, too; but I don't know that a PR is the best place for it. Any advice? |
Is your current plugin code available somewhere on github? |
@felixfontein not yet, but I just found the appropriate documentation, so I'll do that soon. |
You can also create a new repository in your own github account and put it there; but forking and creating a PR for getnikola/plugins is of course fine, too. |
@felixfontein It's not done, but here's what I'm ultimately working on: https://github.com/anderbubble/nikola-postcast |
Both PR and your plugin look fine to me. (In the plugin, you seem to ignore the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@felixfontein thanks for the comments. I've created anderbubble/nikola-postcast#5 and anderbubble/nikola-postcast#6 to track your suggestions. |
Apropos: do you want to leave this PR open a bit more, or do you want to see it merged soon? |
Up to you. I'm running out of a fork that has it merged already. ;) But if
it were merged upstream, one less divergence for me.
Biggest problem with it, I suspect, is that it doesn't update the atom
generator in kind. But I'm not using it, either, so I wouldn't have a
built-in test case, either.
…On Sat, Mar 11, 2017, 4:13 PM Felix Fontein ***@***.***> wrote:
Apropos: do you want to leave this PR open a bit more, or do you want to
see it merged soon?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2686 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAVYVhFLeCsEDUDg6TL6oi_RtpJ1pv_pks5rkyp-gaJpZM4MMprK>
.
|
Merged because it can be useful for others, and Atom works completely differently anyway. |
Pull Request Checklist
Description
I split actual feed generation into
generic_rss_feed
and writing intoutils.rss_writer
, both of which are now called from the (much smaller)generic_rss_renderer
. This allows plugins to usegeneric_rss_feed
, modify the resultant feed, and then write it out withutils.rss_writer
.In my use case, I'm building a "postcast" plugin to generate podcasts from posts. I want to add iTunes-specific tags to the podcast RSS feeds, but I didn't like the feeling of bolting domain-specific tags into the central rss renderer. My WIP plugin can now use the upstream code, but modify the result before it's written out to a file.
I've further made a small change to
ExtendedItem
to make it easier to subclass, as I use subclasses ofExtendedRSS2
andExtendedItem
to actually apply the iTunes tags.