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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supress warning issued for redirect pages #8347

Merged
merged 2 commits into from Aug 19, 2020

Conversation

SeekingMeaning
Copy link
Contributor

This is a 馃悰 bug fix.

Summary

This updates lib/jekyll/commands/doctor.rb to skip redirect pages (from the jekyll-redirect-from plugin) when checking for permalink conflicts

Context

Closes #8346

@ashmaroli
Copy link
Member

I think having a separate :allow_used_permalink? method should be safe.
If there are users facing this issue with another plugin, we can ask them to write a local plugin that redefines the said :allow_used_permalink? method to suit their need..

def allow_used_permalink?(item)
  defined?(JekyllRedirectFrom) && item.is_a?(JekyllRedirectFrom::RedirectPage)
end

Additionally, this change definitely needs a test. You can add another Cucumber scenario like before.

@SeekingMeaning
Copy link
Contributor Author

I think having a separate :allow_used_permalink? method should be safe.

Just making sure, where should this method be added?

@ashmaroli
Copy link
Member

where should this method be added?

It can be a private class method in Jekyll::Commands::Doctor

@ashmaroli
Copy link
Member

Thank you for the quick turnarounds @SeekingMeaning
I would like the cucumber scenario to more resemble a real-world use-case.

redirect_from ["redirect.html", "redirect.html"] doesn't seem clear enough to me.

If you're able to change it, great! Otherwise, I shall update this branch when I can come up with a nice scenario.

@ashmaroli
Copy link
Member

Thank you @SeekingMeaning
@jekyllbot: merge +minor

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

Successfully merging this pull request may close these issues.

Warning issued for redirect pages
3 participants