-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Improve logging of generators and writer loaders #2821
Improve logging of generators and writer loaders #2821
Conversation
This improves the logging of get_generator_classes() to provide the source of the generator (either internal or from a plugin). It also reviews the code to make it clearer (in my opinion, but I'm biased ;)), renames get_writer() to _get_writer_class() for consistency with the renamed _get_generator_classes().
I didn't realise tests were running on my forked branch as well 🤦♂️ Good, first mistake done. I'll fix that and come back, sorry! |
I'll look in more detail later but wanted to mention the logging scheme we use: Use |
Sorry, I should have noticed that part on the contribution document you mentioned. I will change the PR accordingly |
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.
Looks good, thanks 👍
Many thanks to @lgiordani for the enhancements and to @avaris for reviewing. ✨ |
Thanks to AutoPub, these changes have been released as Pelican v4.5.2. 🎉 |
Thanks both! I learned more than I expected 🎉 |
discovered_generators.extend( | ||
[(generator, receiver.__module__) for generator in values] | ||
) |
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.
Is this meant to be inside of the if not isinstance(...)
check? If values
is already an iterable from get_generators.send()
call then this code will never run (and they won't be added to discovered_generators
).
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.
Oh, you are right. I missed the indent level in the diff 🤦 .
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.
That was my guess! 😢 I think the old behavior just needs to de-dent these three lines!
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.
Oh, snap, sorry guys, such a stupid mistake. I'll send a fix asap
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.
Pkgsrc changes : * updated dependencies : removed py-six and added py-rich Upstream changes : - Pelican 4.5.1 : * Refactor intra-site link discovery in order to match more permissively * Fix plugins running twice in auto-reload mode * Add notice to use from pelican import signals instead of import pelican.signals - Pelican 4.5.2 : Improve logging of generators and writer loaders - Pelican 4.5.3 : Fix a mistake made in PR #2821 (getpelican/pelican#2821) - Pelican 4.5.4 : Replace plugin definitions in settings with string representations after registering, so they can be cached correctly. - Pelican 4.6.0 : * Add new URL pattern to PAGINATION_PATTERNS for the last page in the list * Speed up livereload Invoke task via caching * Ignore None return value from get_generators signal * Relax dependency minimum versions and remove upper bounds - Pelican 4.7.0 : * Improve default theme rendering on mobile and other small screen devices * Add support for hidden articles * Improve word count behavior when generating summary CJK & other locales * Add progress spinner during generation and richer logging, both via Rich * Invoke tasks serve and livereload now auto-open a web browser pointing to the locally-served web site * Support some date format codes used by ISO dates * Document how to add a new writer - Pelican 4.7.1 : * Extend rich logging to server component * Fix an issue where metadata flagged to be discarded was being cached * Adjust suffix in server to allow redirection when needed * Add MIME types for web fonts * Distribute sample data used to run tests * Add Python 3.10 to test matrix - Pelican 4.7.2 : * Fix incorrect parsing of parameters specified via -e / --extra-settings option flags * Add categories.html template to default theme * Document how to use plugins to inject content - Pelican 4.8.0 : * Use JSON values for extra settings in Invoke tasks template * Add content tag for links, which can help with things like Twitter social cards * Improve word count behavior when generating summary
This improves the logging of
get_generator_classes()
to provide the source of the generator (either internal or from a plugin). It also reviews the code to make it clearer (in my opinion, but I'm biased 😉), renamesget_writer()
to_get_writer_class()
for consistency with the renamed_get_generator_classes()
. Both are purely internal methods so I think it makes sense to signal it with the initial underscore.Aside from cosmetic changes this doesn't affect the Pelican loading mechanism at all.
This is my first PR for this project, so please help me double checking that everything was done according to the guidelines.
Pull Request Checklist