-
Notifications
You must be signed in to change notification settings - Fork 311
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
Page context plugin #141
base: master
Are you sure you want to change the base?
Page context plugin #141
Conversation
Hi Johannes, Just a quick note that i'll try to take a look at this over the weekend — sorry about the delayed response here. Cheers, |
I appreciate your work for this project very much, and have learned to be patient in open source development, since I'm well aware, that there are more pressing issues in everybodies life than such a project. |
@@ -75,16 +75,11 @@ def context(self, data=None, extra=None): | |||
""" |
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.
I think we should refactor this method. data
is passed, but we never use it. extra
is never passed. How about we remove those altogether?
Cheers,
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.
Sure, I had left it in because somebody else might rely on them being there. But that seems overly cautious.
Hey @jammon , I added some inline comments. Most of them are rather minor. The two major things are:
Cheers, |
Thank you for in-depth comments, I agree with all of them. |
Hey @jammon Just a quick note that this week / the next one have / are going to be very busy on my end. I'll try to review this but I can't promise anything just yet. Cheers, |
@krallin No problem, currently I have more than enough work to do as well. |
Ripped parsing of page context out of
Page
and put it into a plugin. With tests and documentation.There are different styles of metadata, the plugin tries to guess what is appropriate.
Multimarkdown style is implemented but not used. In the future the style should be up for configuration.