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

Incorporate relative_url within post_url tag #7589

Merged
merged 2 commits into from Mar 29, 2019

Conversation

ashmaroli
Copy link
Member

  • This is a 🙋 feature or enhancement.
  • I've added tests
  • I've adjusted the documentation (via spec.post_intall_message)

Summary

With this, {% post_url 2019-03-27-hello %} will return /blog/2019/03/27/hello.html for a site configured with baseurl: blog

BREAKING-CHANGE

Context

Resolves #7353

@ashmaroli ashmaroli added the breaking-change Affect current behavior label Mar 27, 2019
@ashmaroli ashmaroli requested review from a team March 27, 2019 14:21
@update-docs
Copy link

update-docs bot commented Mar 27, 2019

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would update some of our documentation based on your changes.

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Ashwin 🥇

@ashmaroli
Copy link
Member Author

@DirtyF I've documented this change within the gemspec's post_install_message. Isn't that enough?

@chrisfinazzo
Copy link
Contributor

chrisfinazzo commented Mar 28, 2019

As this is a breaking change for nearly everything, would it be appropriate to put this info in a warning block to remind people they will need to fix their links? 🤔

@ashmaroli
Copy link
Member Author

be appropriate to put this info in warning block

@chrisfinazzo Sounds fair. I'll add a warning block to the upgrading docs

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on the feature,

No need for a red warning in the docs, it's a major bump, so you should expect breaking changes.

@chrisfinazzo
Copy link
Contributor

@DirtyF, I guess... 👌🏼

My only point there was because this change affects a fundamental behavior that has worked in a certain way for so so so long, it feels half-assed to not highlight this since almost everyone will encounter it at some point.

@DirtyF
Copy link
Member

DirtyF commented Mar 29, 2019

@chrisfinazzo we have a post-install message for everyone to see.

People might get confused though because the link path will change and this can remained unnoticed because we won't output a warning in the console, and not everyone has automatic link validation with tools link htmlproofer.

@DirtyF
Copy link
Member

DirtyF commented Mar 29, 2019

@jekyllbot: merge +major

@jekyllbot jekyllbot merged commit 2591f33 into jekyll:master Mar 29, 2019
jekyllbot added a commit that referenced this pull request Mar 29, 2019
@mattr-
Copy link
Member

mattr- commented Mar 29, 2019

I appreciate that there's a warning for this feature now. Not everybody is going to read the post install message. With a change like this, it's important to highlight it in as many ways as possible and I'm glad that's what we're doing here. Thanks everybody! ❤️

@ashmaroli ashmaroli deleted the post-url-tag-with-baseurl branch April 28, 2019 15:36
cat-in-136 added a commit to cat-in-136/blog that referenced this pull request Dec 29, 2019
…of imcompatibility between jekyll3.x and 4.x.

>  * Our `link` tag now comes with the `relative_url` filter incorporated into it.
>    You should no longer prepend `{{ site.baseurl }}` to `{% link foo.md %}`
>    For further details: jekyll/jekyll#6727
>
>  * Our `post_url` tag now comes with the `relative_url` filter incorporated into it.
>    You shouldn't prepend `{{ site.baseurl }}` to `{% post_url 2019-03-27-hello %}`
>    For further details: jekyll/jekyll#7589
@jekyll jekyll locked and limited conversation to collaborators Apr 27, 2020
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.

feat: implement relative_url fil in post_url tag
5 participants