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

Add atom feed. #225

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

sunnyrjuneja
Copy link

Recently found practicing ruby and really enjoy it. Would love to add it to my rss/atom reader.

Also, fyi, the open source version of practicing-ruby-web doesn't seem to be inline with www.practicingruby.com since it requires subscription to view any articles.

@practicingruby
Copy link
Member

@whatasunnyday That's a problem with the data being outdated. Do something like this to fix it for the moment: Article.all.each { |a| a.update_attribute(:status, :public) }. A PR to fix the snapshot would be welcome!

@practicingruby
Copy link
Member

Going to deploy this from a branch and kick the tires, thanks.

@practicingruby
Copy link
Member

@whatasunnyday:
I had to tweak this a little bit, but it's now running on https://practicingruby.com/archives/public.atom

I'm not sure it's going to be feasible for us to give full content feeds, because it seems likely that RSS readers won't render things like code samples correctly, and after downloading a couple random feed readers it appears that's the case.

The dates on the feeds also appear to be wrong, not sure why because published_time is the right field to look at, but somehow it looks like the timestamps match the updated_at value instead.

You can try this out yourself and see if you also run into these problems, but figuring out these issues is what has prevented me from implementing a feed so far. 😢

@practicingruby
Copy link
Member

@sunnyrjuneja
Copy link
Author

Hey @sandal. Thanks for the quick response.

A PR to fix the snapshot would be welcome!

I'll send a PR for other issue sometime today or tomorrow.

I'm not sure it's going to be feasible for us to give full content feeds, because it seems likely that RSS readers won't render things like code samples correctly, and after downloading a couple random feed readers it appears that's the case.

I mostly use rss/atom readers to get notified when new articles are published and to track articles that I have read/not read. Most readers have an option to jump out of the reader and go straight to the article. This is what I was planning to do.

The dates on the feeds also appear to be wrong, not sure why because published_time is the right field to look at, but somehow it looks like the timestamps match the updated_at value instead.

This is weird. I'll check this out.

@practicingruby
Copy link
Member

@whatasunnyday:

I mostly use rss/atom readers to get notified when new articles are published and to track articles that I have read/not read. Most readers have an option to jump out of the reader and go straight to the article. This is what I was planning to do.

OK, then instead of the full article body, maybe we can use the short one line summary as the body? This will keep the feed size small and prevent readers from experiencing broken feeds.

We may also want to add autodiscovery support, too.

Please pull my commits from the atom branch on this repo into your fork and then add some commits when you're ready.

@sunnyrjuneja
Copy link
Author

Cool, will do.

@sunnyrjuneja
Copy link
Author

Hey @sandal,

I had to tweak this a little bit, but it's now running on https://practicingruby.com/archives/public.atom

Just heads up, doesn't look like the link is working:
6229774

The dates on the feeds also appear to be wrong, not sure why because published_time is the right field to look at, but somehow it looks like the timestamps match the updated_at value instead.

If I'm not mistaken, it looks like its currently correct:

Article.last
  Article Load (1.6ms)  SELECT "articles".* FROM "articles" ORDER BY "articles"."id" DESC LIMIT 1
=> #<Article id: 69, subject: "Spiking is not cowboy coding", body: "> **NOTE:** This is one of [four lessons\r\n> learned...", status: "public", mailchimp_campaign_id: nil, published_time: "2012-07-26 00:00:00", created_at: "2012-07-25 16:06:15", updated_at: "2014-10-22 17:03:40", issue_number: "4.12.4", volume_id: 4, collection_id: 3, summary: "The thing that cowboy coding and an experimental sp...", slug: "spiking-is-not-cowboy-coding", recommended: false>
 <entry>
    <id>tag:localhost,2005:ArticleDecorator/69</id>
    <published>2012-07-26T00:00:00Z</published>
    <updated>2014-10-22T17:03:40Z</updated>
    <link rel="alternate" type="text/html" href="http://localhost:3000/articles/spiking-is-not-cowboy-coding"/>
    <title>Spiking is not cowboy coding</title>
    <content>The thing that cowboy coding and an experimental spike have in common is that they relax many of the quality rules that we impose upon ourselves. But because the former offers only chaos, and the latter is meant to be a well-constrained application of "organized chaos", it is important to know the difference between the two. This article shows examples of both extremes as well as something in the middle.</content>
    <summary>The thing that cowboy coding and an experimental spike have in common is that they relax many of the quality rules that we impose upon ourselves. But because the former offers only chaos, and the latter is meant to be a well-constrained application of "organized chaos", it is important to know the difference between the two. This article shows examples of both extremes as well as something in the middle.</summary>
    <author>
      <name>Practicing Ruby</name>
    </author>
  </entry>

Correct me if I'm wrong but published time is 2012-07-26 is set in the atom feed and published_time in the database. Did I misinterpret your comment?

OK, then instead of the full article body, maybe we can use the short one line summary as the body?

Done.

We may also want to add autodiscovery support, too.

Done.

Want me to rebase?

@sunnyrjuneja
Copy link
Author

A PR to fix the snapshot would be welcome!

Did you want me to alter the dumb.sql directly or update the attribute in the rake task?

@practicingruby
Copy link
Member

Just heads up, doesn't look like the link is working.

I'm currently deployed from master. I'll redeploy this once I get chance to review, thanks.

@sunnyrjuneja
Copy link
Author

@sandal Hey, just wanted to check in to see if there's anything I need to improve to get this PR merged. I know you're busy so no pressure.

@practicingruby
Copy link
Member

I just haven't had a chance to review yet. The only think I might be concerned about is if this is using the updated_at field in any way, it's going to produce awkward timestamps, because I don't think you necessarily would want to see something like a single character typo fix show up as something "new". For now it would probably be best to make sure we use only the published_time, and maybe some time later I'll add an extra field like revised_time which would be only updated when major revisions are done.

But I was going to tweak these things myself when I reviewed, so it's up to you whether you want to look at it now or just wait until I've freed up some time.

@sunnyrjuneja
Copy link
Author

I'll just wait. There's absolutely no reason to rush.

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

Successfully merging this pull request may close these issues.

2 participants