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

Similar plugin #3

Open
ashwinvis opened this issue Apr 1, 2020 · 4 comments
Open

Similar plugin #3

ashwinvis opened this issue Apr 1, 2020 · 4 comments

Comments

@ashwinvis
Copy link

This plugin is very similar to:

which I forked recently and added async functionality to in:

There may be ideas in the implementation of pelican-planet which could be useful for webring.

@davidag
Copy link
Collaborator

davidag commented Apr 2, 2020

Thank you for letting us know about that plugin and your async implementation! They really look similar! 😅

I'll try to review both and compare features (at first sight, it looks like there are some important differences). Maybe we could merge them in one plugin supporting all use cases (shouldn't be too many), what do you think @ashwinvis ?

@ashwinvis
Copy link
Author

That sounds like a great idea 👍. I think it should be done incrementally. If I may propose:

  1. Could you structure the plugin using a class, rather than using module level globals. Also the __init__.py can take care of registering the plugin and parsing the config variables. This is what was done in pelican-planet.
  2. I can convert the function

def get_feed_html(feed_url):

into something like https://github.com/ashwinvis/pelican-planet/blob/4c90e8e66c277774b841c38a01e062cc7a17bd44/pelican_planet/planet.py#L57-L102

  1. Combine the modules

@davidag
Copy link
Collaborator

davidag commented Apr 2, 2020

Great information @ashwinvis, I'd like to understand first the high level differences and then plan how to implement the differences.

I think it's important to try to keep the configuration as equivalent as possible, to ease any migration between plugins. Also, I think it's better to improve webring than planet because it already has a repo in pelican-plugins and the name seems more descriptive, in my opinion.

About your proposal:

  • I'm not sure having a class is necessary when there is no inheritance, nor the object will be instantiated more than once. I think a module is good enough in this case.
  • I'm no asyncio expert, so I think your plugin can be of great use there. By the way, have you noticed any speed improvement?

Thanks!

@ashwinvis
Copy link
Author

I'm not sure having a class is necessary when there is no inheritance, nor the object will be instantiated more than once. I think a module is good enough in this case.

It was useful for testing with different parameters (https://framagit.org/bochecha/pelican-planet/-/blob/master/pelican_planet/tests/test_planet.py).

I'm no asyncio expert, so I think your plugin can be of great use there. By the way, have you noticed any speed improvement?

Substantial!

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

2 participants