-
-
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
Adding missing tests for truncate_html_words() #2918
Adding missing tests for truncate_html_words() #2918
Conversation
Added support for Jinja2 generated end marker when the passed variable is an instance of Markup. This makes the function compatible with Jinja2's environments where autoescape is enabled.
I am looking at the failed tests and I am puzzled re: how my change could have affected these three tests. Moreover, when I run |
There are two CI failures. The first one is indeed unrelated to your changes and is instead caused by a newer version of FeedGenerator, for which we have not yet updated our functional test output. The second one is due to code style conformance checks. Did you follow the development docs and run |
… and the more I look at the diff caused by the latest version of FeedGenerator, the more I wonder whether FeedGenerator 1.9.2 has changed feed output in an undesirable manner. Issue filed: getpelican/feedgenerator#30 🤔 |
Yeah, I also found that FeedGenerator has been updated on Aug 18 and the diff I made shows that the difference is actually the missing |
@justinmayer , I did fix linting issues (there is a commit just before your comment). I was relying on my Vim being good with flake8 rules, but it was not, so when I've seen the linting errors, I quickly amended the file :). |
@avaris correctly pointed out that I am tapping into an internal function ( |
@galaxy4public: That seems to me a reasonable path forward on this topic. Would you please proceed as you suggested above? |
@justinmayer , will do that (most likely this coming weekend since I am currently smashed with work :( ) |
Hi @galaxy4public. Just checking in regarding your pull request. Do you think you could find some time to move it forward so we can get it merged? |
@justinmayer, sorry, I was overloaded with other work lately. Should be able to work on this by the end of this week. |
@justinmayer, I've updated the PR and it should be ready to merge. |
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.
Many thanks for adding these tests, @galaxy4public! 🥇
Added support for Jinja2 generated end marker when the passed variable
is an instance of Markup. This makes the function compatible with
Jinja2's environments where autoescape is enabled.
Pull Request Checklist
Resolves: #2917