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

Feed-related code is mixed into articles and settings code #2695

Closed
kurtmckee opened this issue Feb 23, 2020 · 7 comments
Closed

Feed-related code is mixed into articles and settings code #2695

kurtmckee opened this issue Feb 23, 2020 · 7 comments

Comments

@kurtmckee
Copy link
Contributor

I'm reviewing Pelican's feed-related code and I've found that it is mixed into ArticlesGenerator.generate_feeds() (in generators.py) and configure_settings() (in settings.py, which generates a warning if SITEURL isn't set)

I'm interested in isolating the feed-related code into its own generator and contributing the changes back to the project but I'd like to confirm whether this work would be considered for inclusion.

Please let me know. Thanks!

@avaris
Copy link
Member

avaris commented Feb 23, 2020

What would be the benefit of that? Currently, feeds are for articles and their generation is part of article related output process. So it feels natural to have them in ArticlesGenerator.

PS: I would be very opposed to moving that warning out of configure_settings. The job of that function is to perform certain adjustments and sanity checks on the defined settings upon initialization and that warning very much belongs there.

@kurtmckee
Copy link
Contributor Author

kurtmckee commented Feb 23, 2020

@avaris, I may have misunderstood the point of ArticlesGenerator.

My goal is to improve and isolate the XML feed writer and to introduce a JSON feed writer for my site.

However, Pelican explicitly discards all but the first Writer it finds (I'm looking at __init__:Pelican.get_writer()) so feed generation is hard-coded into the Articles code so that two distinct types of writers (HTML files and XML feeds) can still be called.

It looks like I can separate the HTML and XML feed writers into separate Writer objects and then rely solely on signals for both of those and for a future JSON feed writer plugin.

Are there better ways to add additional writers in this situation? I can write a plugin to accomplish all of this but it will require monkey-patching Pelican's internals instead of building on its built-in functionality. That's why I'm currently thinking that I could improve how Pelican's own writers interact with generators, to pave the way for smoother plugin development in this area.

@justinmayer
Copy link
Member

Link to JSON-feed-related issue on FeedGenerator repository: getpelican/feedgenerator#11

@avaris
Copy link
Member

avaris commented Feb 23, 2020

Yes, current design requires that there can only be one writer and base writer delegates a lot of the feed related tasks to feedgenerator. So it's probably best to extend feedgenerator with JSON feed option (^ issue linked by @justinmayer) amd provide that functionality in pelican core alongside with ATOM and RSS.

@justinmayer
Copy link
Member

@kurtmckee: Any follow-up thoughts regarding this issue?

@justinmayer
Copy link
Member

Hi Kurt. As @avaris suggested, if you would like to extend FeedGenerator to support JSON and then submit a PR to Pelican that uses that support, your contributions would be most welcome. Hopefully that answers your original question. I look forward to seeing what you come up with! ✨

@kurtmckee
Copy link
Contributor Author

kurtmckee commented Apr 30, 2020 via email

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

No branches or pull requests

3 participants