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

Mentionify only relevant docs or pages #56

Merged
merged 2 commits into from
Mar 14, 2018

Conversation

ashmaroli
Copy link
Member

  • add a check to ensure that the content passed to HTML::Pipeline::MentionFilter contains the given mention_username_pattern.
  • this avoids having to process docs or pages that contain @ only in the <head> element

@pathawks
Copy link
Member

pathawks commented Mar 5, 2018

Do you have any benchmarks?

@ashmaroli
Copy link
Member Author

Do you have any benchmarks?

Not really, but Jekyll's official docs build faster with this patch..

@ghost
Copy link

ghost commented Mar 5, 2018

@pathawks As far as my own flamegraph tests with the docs go, emojify and mentionify always take up a significant chunk of processing time, so I think anything to slim that down is a good idea.

@DirtyF
Copy link
Member

DirtyF commented Mar 5, 2018

Build time seems to take 1s less with this patch on our docs on macOS.

@ashmaroli
Copy link
Member Author

Based on script/stackprofat Jekyll:

TOTAL    (pct)     SAMPLES    (pct)     FRAME
  954  (31.2%)          0    (0.0%)     Jekyll::Mentions.mentionify   (jekyll-mentions-1.2.0)
  843  (19.1%)         10    (0.2%)     Jekyll::Mentions.mentionify   (this-PR)

@ashmaroli
Copy link
Member Author

ping: Travis needs a restart..

@DirtyF
Copy link
Member

DirtyF commented Mar 14, 2018

@jekyllbot: merge +minor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants