Skip to content
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

Provide copyright field in RSS feed #2676

Merged
merged 7 commits into from Feb 26, 2017

Conversation

@anderbubble
Copy link
Contributor

@anderbubble anderbubble commented Feb 19, 2017

I'm working on enhancing Nikola to support publishing a podcast RSS feed. Most of this work is going into a "postcast" plugin I'm working on, and will submit or publish eventually; but some of what I'm trying to do requires support in Nikola itself.

This PR aims to support providing a copyright field in RSS feeds as recommended by iTunes connect.

Other related PRs I'm working on:

  • #2675 (explicit / static guids)
  • #2674 (iTunes rss tags)

I'd also be interested in direction for where and how to publish a plugin, when that's ready.

nikola/nikola.py Outdated
@@ -1660,6 +1660,11 @@ def generic_rss_renderer(self, lang, title, link, description, timeline, output_
language=lang
)

copyright = self.config['CONTENT_FOOTER'](lang)
Copy link
Member

@Kwpolska Kwpolska Feb 19, 2017

Choose a reason for hiding this comment

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

The existence of copyright should be configurable, and CONTENT_FOOTER might not be the right place to take it from.

Loading

@anderbubble
Copy link
Contributor Author

@anderbubble anderbubble commented Feb 19, 2017

First, thank you for reviewing this pull request!

I'm happy to address the requested changes. I'll change copyright to an argument for the relevant function.

Do you have a recommendation or a better general-purpose place to take that from?

Loading

@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Feb 19, 2017

I’m afraid there isn’t anything immediately suitable. You could always create a new config variable — say, RSS_COPYRIGHT, and maybe RSS_COPYRIGHT_PLAIN.

Loading

@anderbubble anderbubble changed the title Provide copyright field in RSS feed [WIP] Provide copyright field in RSS feed Feb 25, 2017
Taking in recommendations from getnikola#2676

* Added RSS_COPYRIGHT and RSS_COPYRIGHT_PLAIN
* Added RSS_COPYRIGHT_FORMATS (like CONTENT_FOOTER_FORMATS)
@anderbubble
Copy link
Contributor Author

@anderbubble anderbubble commented Feb 25, 2017

@Kwpolska I think I've addressed your concerns. Let me know how this looks now. (I'm happy to squash the commits, too, if that's useful.)

edit: It appears that codacy doesn't follow how config variables become translatable? Or did I do something wrong? (It appears to work in my environment.)

Loading

Copy link
Member

@Kwpolska Kwpolska left a comment

LGTM, but please address the minor style remarks, add yourself to AUTHORS.txt and this change to the top of CHANGES.txt.

Loading

@@ -1240,3 +1240,9 @@ GLOBAL_CONTEXT = {}
# GLOBAL_CONTEXT as parameter when the template is about to be
# rendered
GLOBAL_CONTEXT_FILLER = []

# A simple copyright tag for inclusion in RSS feeds that works just
Copy link
Member

@Kwpolska Kwpolska Feb 25, 2017

Choose a reason for hiding this comment

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

Put it under CONTENT_FOOTER.

Loading

nikola/nikola.py Outdated
@@ -1660,6 +1675,11 @@ def generic_rss_renderer(self, lang, title, link, description, timeline, output_
language=lang
)

if copyright_ is True:
Copy link
Member

@Kwpolska Kwpolska Feb 25, 2017

Choose a reason for hiding this comment

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

Just do if copyright_: here. Add a comment for future readers that the second if is checking if any copyright info is set.

Loading

@anderbubble
Copy link
Contributor Author

@anderbubble anderbubble commented Feb 25, 2017

Loading

anderbubble added 4 commits Feb 25, 2017
I was using True before to decide whether RSS_COPYRIGHT* should
be read from config; but None is probably more idiomatic, and
still supports specifying an explicit value as an argument.
@anderbubble
Copy link
Contributor Author

@anderbubble anderbubble commented Feb 25, 2017

@Kwpolska I can rebase on top of current master if you'd like, to resolve the conflict; should I squash as well, or leave as-is?

Loading

@anderbubble anderbubble changed the title [WIP] Provide copyright field in RSS feed Provide copyright field in RSS feed Feb 25, 2017
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Feb 25, 2017

No need to squash.

Loading

@Kwpolska Kwpolska merged commit e6c0164 into getnikola:master Feb 26, 2017
0 of 3 checks passed
Loading
@Kwpolska
Copy link
Member

@Kwpolska Kwpolska commented Feb 26, 2017

Merged. Thanks for contributing to Nikola! 🎉

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants