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

Improve docs feed #2192

Merged
merged 2 commits into from
Apr 2, 2014
Merged

Improve docs feed #2192

merged 2 commits into from
Apr 2, 2014

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Apr 1, 2014

@parkr: let me know if you need me to make any changes.

I personally find it useful to have the generator property, but it's just cosmetic.

Also for the feed image, Firefox shows it right, Chrome doesn't. Keep in mind that these must be in gh-pages for the image link to work :)

I used https://github.com/jekyll/jekyll-logo/blob/master/jekyll-logo-light-transparent.png and resized it to 144x73 since the specs say that the maximum image width is 144px.

<description><![CDATA[ {{ post.content }} ]]></description>
<title>{{ post.title | xml_escape}}</title>
<link>http://jekyllrb.com{{ post.url }}</link>
<pubDate>{{ post.date | date: "%a, %d %b %Y %H:%M:%S %z" }}</pubDate>
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps you can use the date_to_rfc822 filter here instead of this long date format string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah makes sense. I didn't touch the date stuff :)

Copy link
Member

Choose a reason for hiding this comment

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

Fair enough. Might as well change it up to increase readability, though. There are a couple more instances that can be updated too, if you don't mind. (Line 18 and 19)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added it, thanks @troyswanson

@troyswanson
Copy link
Member

👍 Looks good! 👏

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Apr 1, 2014

Unfortunately, we can't make the feed valid since GitHub Pages doesn't allow us to use custom plugins...

http://validator.w3.org/feed/check.cgi?url=http%3A%2F%2Fjekyllrb.com%2Ffeed.xml

I personally use this. Not sure if there's another better or simpler way to do it.

@troyswanson
Copy link
Member

I'm confused. It says it is valid for me.

@XhmikosR
Copy link
Contributor Author

XhmikosR commented Apr 1, 2014

Hmm I should have said, the feed doesn't have any warnings I suppose.

@troyswanson
Copy link
Member

Meh, if it's valid, it's valid! 👍

@parkr
Copy link
Member

parkr commented Apr 2, 2014

@XhmikosR Would you mind doing a quick rebase? ❤️

* add `generator` property
* add image property
* use `xml_escape` instead of `![CDATA]`
* use `date_to_rfc822` instead of the date format string (thanks @troyswanson!)
* fix self reference url
* switch to `isPermaLink` for the `guid`
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Apr 2, 2014

@parkr: done!

parkr added a commit that referenced this pull request Apr 2, 2014
@parkr parkr merged commit 1437fb2 into jekyll:master Apr 2, 2014
parkr added a commit that referenced this pull request Apr 2, 2014
@XhmikosR XhmikosR deleted the docs-feed branch April 2, 2014 18:39
@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017
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.

None yet

4 participants