Move design and code review to discussion subpage.
Amitai Schlair
parent
0db80d453f
commit
b4a41c0595
|
|
@ -10,10 +10,7 @@ also have lots more metadata.
|
||||||
[[!template id=gitbranch branch=schmonz/fancypodcast author="[[schmonz]]"]]
|
[[!template id=gitbranch branch=schmonz/fancypodcast author="[[schmonz]]"]]
|
||||||
[[!tag patch]]
|
[[!tag patch]]
|
||||||
|
|
||||||
In summary, the branch preserves ikiwiki's existing podcast behavior,
|
Nothing new on the branch since 2013/07/21 merge to `master`.
|
||||||
adds more featureful behavior, and has been tested to work well in
|
|
||||||
some common podcatchers. I believe it is ready for integration.
|
|
||||||
--[[schmonz]]
|
|
||||||
|
|
||||||
## Features
|
## Features
|
||||||
|
|
||||||
|
|
@ -34,53 +31,6 @@ Episode description|(./) |(./) |(./) |
|
||||||
Episode enclosure |(./) |(./) |(./) |(./)
|
Episode enclosure |(./) |(./) |(./) |(./)
|
||||||
"""]]
|
"""]]
|
||||||
|
|
||||||
## Design
|
|
||||||
|
|
||||||
7. For each fancy podcast episode, write a blog post containing
|
|
||||||
`\[[!meta enclosure="WikiLink/to/media.mp3"]]`. (Don't specify
|
|
||||||
more than one enclosure -- but if you do, last one wins.)
|
|
||||||
7. When rendering to HTML (single-page or inlined), append a link
|
|
||||||
to the media file.
|
|
||||||
7. When rendering to RSS/Atom, the text is the entry's content and
|
|
||||||
the media file is its enclosure.
|
|
||||||
7. Don't break simple podcasts in pursuit of fancy podcasts.
|
|
||||||
|
|
||||||
## Implementation
|
|
||||||
|
|
||||||
### Completed
|
|
||||||
|
|
||||||
* Cover the existing simple podcast behavior with tests.
|
|
||||||
* Add an `enclosure` field to [[plugins/meta]] that expands the
|
|
||||||
given [[ikiwiki/WikiLink]] to an absolute URL (feed enclosures
|
|
||||||
pretty much need to be, and the reference feeds I've looked at
|
|
||||||
all do this).
|
|
||||||
* Write failing tests for the desired single-page and inlined
|
|
||||||
HTML behavior, then make them pass by adding enclosure stanzas
|
|
||||||
to `{,inline}page.tmpl`.
|
|
||||||
* Write failing tests for the desired RSS/Atom behavior, then make
|
|
||||||
them pass via changes to `{atom,rss}item.tmpl` and [[plugins/inline]].
|
|
||||||
* Match feature-for-feature with
|
|
||||||
[tru_podcast](http://www.rainskit.com/blog/542/tru_podcast-a-podcasting-plugin-for-textpattern)
|
|
||||||
(what [[schmonz]] will be migrating from).
|
|
||||||
* Enrich [feed metadata](http://cyber.law.harvard.edu/rss/rss.html)
|
|
||||||
by catching up `rsspage.tmpl` to `atompage.tmpl`.
|
|
||||||
* Verify that [[plugins/more]] plays well with fancy podcasts.
|
|
||||||
* Verify that the feeds validate.
|
|
||||||
* Subscribe to a fancy feed in some common podcatchers and verify
|
|
||||||
display details against a reference podcast.
|
|
||||||
* Verify smooth transitions for two common use cases (see testing
|
|
||||||
details below).
|
|
||||||
* Code review: don't add enclosure divs unless we have enclosures.
|
|
||||||
* Code review: genericize download link for more use cases.
|
|
||||||
* Code review: don't confuse old readers with Atom names in RSS.
|
|
||||||
* Code review: instead of hacking back to `$link`, just provide it.
|
|
||||||
* Code review: show author in addition to feedname, if different.
|
|
||||||
|
|
||||||
### Must-have (for [[schmonz]], anyway)
|
|
||||||
|
|
||||||
* Think carefully about UTF-8.
|
|
||||||
* Verify that _all_ the tests pass (not just my new ones).
|
|
||||||
|
|
||||||
## Migration
|
## Migration
|
||||||
|
|
||||||
### Upgrading within ikiwiki: from simple to fancy
|
### Upgrading within ikiwiki: from simple to fancy
|
||||||
|
|
@ -228,125 +178,6 @@ it with ikiwiki instead.
|
||||||
iTunes) alongside the RSS/Atom ones in [[plugins/inline]].
|
iTunes) alongside the RSS/Atom ones in [[plugins/inline]].
|
||||||
* Support Apple's "enhanced podcasts" (if they're still relevant).
|
* Support Apple's "enhanced podcasts" (if they're still relevant).
|
||||||
|
|
||||||
### code review
|
|
||||||
|
|
||||||
+ # XXX better way to compute relative to srcdir?
|
|
||||||
+ my $file = $absurl;
|
|
||||||
+ $file =~ s|^$config{url}/||;
|
|
||||||
|
|
||||||
I don't think ikiwiki offers a better way to do that, because there is
|
|
||||||
normally no reason to do that. Why does it need an url of this form here?
|
|
||||||
--[[Joey]]
|
|
||||||
|
|
||||||
> In all the popular, production-quality podcast feeds I've looked
|
|
||||||
> at, enclosure URLs are always absolute (even when they could be
|
|
||||||
> expressed concisely as relative). [Apple's
|
|
||||||
> example](http://www.apple.com/itunes/podcasts/specs.html#example)
|
|
||||||
> does too. So I told \[[!meta]] to call `urlto()` with the third
|
|
||||||
> parameter true, which means the \[[!inline]] code here gets an
|
|
||||||
> absolute URL in `$pagestate{$p}{meta}{enclosure}`. To compute the
|
|
||||||
> enclosure's metadata, though, we of course need it as a local path.
|
|
||||||
> I didn't see a less
|
|
||||||
> [ongepotchket](http://www.jewish-languages.org/jewish-english-lexicon/words/1402)
|
|
||||||
> way at the time. If you have a better idea, I'm happy to hear it;
|
|
||||||
> if not, I'll add an explanatory comment. --[[schmonz]]
|
|
||||||
|
|
||||||
>> I would be more comfortable with this if two two different forms of url
|
|
||||||
>> you need were both generated by calling urlto. It'd be fine to call
|
|
||||||
>> it more than once. --[[Joey]]
|
|
||||||
|
|
||||||
>>> Heh, it was even easier than that! (Hooray for tests.) Done.
|
|
||||||
>>> --[[schmonz]]
|
|
||||||
|
|
||||||
+<TMPL_IF HTML5><section id="inlineenclosure"><TMPL_ELSE><div id="inlineenclosure"></TMPL_IF>
|
|
||||||
+<TMPL_IF ENCLOSURE>
|
|
||||||
|
|
||||||
Can't we avoid adding this div when there's no enclosure? --[[Joey]]
|
|
||||||
|
|
||||||
> Sure, I've moved the `<TMPL_IF ENCLOSURE>` check to outside the
|
|
||||||
> section-and-div block for `{,inline}page.tmpl`. --[[schmonz]]
|
|
||||||
|
|
||||||
+<a href="<TMPL_VAR ENCLOSURE>">Download this episode</a>
|
|
||||||
|
|
||||||
"Download this episode" is pretty specific to particular use cases.
|
|
||||||
Can this be made more generic, perhaps just "Download"? --[[Joey]]
|
|
||||||
|
|
||||||
> Yep, I got a little carried away. Done. --[[schmonz]]
|
|
||||||
|
|
||||||
-<TMPL_IF AUTHOR>
|
|
||||||
- <title><TMPL_VAR AUTHOR ESCAPE=HTML>: <TMPL_VAR TITLE></title>
|
|
||||||
- <dcterms:creator><TMPL_VAR AUTHOR ESCAPE=HTML></dcterms:creator>
|
|
||||||
|
|
||||||
This change removes the author name from the title of the rss feed, which
|
|
||||||
does not seem necessary for fancy podcasts. And it is a change that
|
|
||||||
could negatively impact eg, Planet style aggregators using ikiwiki. --[[Joey]]
|
|
||||||
|
|
||||||
> While comparing how feeds render in podcatchers, I noticed that
|
|
||||||
> RSS and Atom were inconsistent in a couple ways, of which this was
|
|
||||||
> one. The way I noticed it: with RSS, valuable title space was being
|
|
||||||
> spent to display the author. I figured Atom's display was the one
|
|
||||||
> worth matching. You're right, of course, that planets using the
|
|
||||||
> default template and somehow relying on the current author-in-the-title
|
|
||||||
> rendering for RSS feeds (but not Atom feeds!) would be broken by
|
|
||||||
> this change. I'm having trouble imagining exactly what would break,
|
|
||||||
> though, since guids and timestamps are unaffected. Would it suffice
|
|
||||||
> to provide a note in the changelog warning people to be careful
|
|
||||||
> upgrading their planets, and to customize `rssitem.tmpl` if they
|
|
||||||
> really prefer the old behavior (or don't want to take any chances)?
|
|
||||||
> --[[schmonz]]
|
|
||||||
|
|
||||||
>> A specific example I know of is updo.debian.net, when used with
|
|
||||||
>> rss2email. Without the author name there, one cannot see who posted
|
|
||||||
>> an item. It's worth noting that planet.debian.org does the same thing
|
|
||||||
>> with its rss feed. (That's probably what I copied.) Atom feeds may
|
|
||||||
>> not have this problem, don't know. --[[Joey]]
|
|
||||||
|
|
||||||
>>> Okay, that's easy to reproduce. It looks like this _might_ be
|
|
||||||
>>> a simple matter of getting \[[!aggregate]] to populate author in
|
|
||||||
>>> `add_page()`. I'll see what I can figure out. --[[schmonz]]
|
|
||||||
|
|
||||||
>>>> Yep, that was mostly it. If the feed entry defines an author,
|
|
||||||
>>>> and the author is distinct from the feed name, we now show `NAME:
|
|
||||||
>>>> AUTHOR`, else just show `NAME` (same as always). In addition,
|
|
||||||
>>>> the W3 feed validator says `<dcterms:creator>` is invalid, so
|
|
||||||
>>>> I replaced it with `<dc:creator>`, and all of a sudden `r2e`
|
|
||||||
>>>> gives me better `From:` headers. With the latest on my branch,
|
|
||||||
>>>> when I generate the same planet as updo and run `r2e` over it,
|
|
||||||
>>>> the names I get in `From:` look like so:
|
|
||||||
|
|
||||||
* `"updo: Junio C Hamano"`
|
|
||||||
* `"updo: Greg Kroah-Hartman"`
|
|
||||||
* `"updo: Eric Raymond: esr"` (article author != feed name, so we get both)
|
|
||||||
* `"updo: Jannis Pohlman: Jannis Pohlmann"` (oops! I tweaked the real updo)
|
|
||||||
|
|
||||||
>>>> --[[schmonz]]
|
|
||||||
|
|
||||||
+++ b/templates/rsspage.tmpl
|
|
||||||
+ xmlns:atom="http://www.w3.org/2005/Atom"
|
|
||||||
+<atom:link href="<TMPL_VAR FEEDURL>" rel="self" type="application/rss+xml" />
|
|
||||||
|
|
||||||
Why is it using atom namespace inside an rss feed? What are the chances
|
|
||||||
every crummy rss reader on earth is going to understand this? I'd put it at
|
|
||||||
about 0%; I doubt ikiwiki's own rss reader understands such a mashup.
|
|
||||||
--[[Joey]]
|
|
||||||
|
|
||||||
> The validator I used (<http://validator.w3.org/feed/>) told me to.
|
|
||||||
> Pretty sure it doesn't make anything work better in the podcatchers
|
|
||||||
> I tried. Hadn't considered that it might break some readers.
|
|
||||||
> Removed. --[[schmonz]]
|
|
||||||
|
|
||||||
+<generator>ikiwiki</generator>
|
|
||||||
|
|
||||||
Does this added tag provide any benefits? --[[Joey]]
|
|
||||||
|
|
||||||
> Consistency with the Atom feed, and of course it trumpets ikiwiki
|
|
||||||
> to software and/or curious humans who inspect their feeds. The tag
|
|
||||||
> arrived only in RSS 2.0, but that's already the version we're
|
|
||||||
> claiming to be, and it's over a decade old. Seems much less risky
|
|
||||||
> than the atom namespace bits. --[[schmonz]]
|
|
||||||
|
|
||||||
>> Sounds ok then. --[[Joey]]
|
|
||||||
|
|
||||||
----
|
----
|
||||||
|
|
||||||
[[merged|done]] --[[Joey]]
|
[[merged|done]] --[[Joey]]
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,162 @@
|
||||||
|
# Round 1
|
||||||
|
|
||||||
|
## Design
|
||||||
|
|
||||||
|
7. For each fancy podcast episode, write a blog post containing
|
||||||
|
`\[[!meta enclosure="WikiLink/to/media.mp3"]]`. (Don't specify
|
||||||
|
more than one enclosure -- but if you do, last one wins.)
|
||||||
|
7. When rendering to HTML (single-page or inlined), append a link
|
||||||
|
to the media file.
|
||||||
|
7. When rendering to RSS/Atom, the text is the entry's content and
|
||||||
|
the media file is its enclosure.
|
||||||
|
7. Don't break simple podcasts in pursuit of fancy podcasts.
|
||||||
|
|
||||||
|
## Implementation
|
||||||
|
|
||||||
|
### Completed
|
||||||
|
|
||||||
|
* Cover the existing simple podcast behavior with tests.
|
||||||
|
* Add an `enclosure` field to [[plugins/meta]] that expands the
|
||||||
|
given [[ikiwiki/WikiLink]] to an absolute URL (feed enclosures
|
||||||
|
pretty much need to be, and the reference feeds I've looked at
|
||||||
|
all do this).
|
||||||
|
* Write failing tests for the desired single-page and inlined
|
||||||
|
HTML behavior, then make them pass by adding enclosure stanzas
|
||||||
|
to `{,inline}page.tmpl`.
|
||||||
|
* Write failing tests for the desired RSS/Atom behavior, then make
|
||||||
|
them pass via changes to `{atom,rss}item.tmpl` and [[plugins/inline]].
|
||||||
|
* Match feature-for-feature with
|
||||||
|
[tru_podcast](http://www.rainskit.com/blog/542/tru_podcast-a-podcasting-plugin-for-textpattern)
|
||||||
|
(what [[schmonz]] will be migrating from).
|
||||||
|
* Enrich [feed metadata](http://cyber.law.harvard.edu/rss/rss.html)
|
||||||
|
by catching up `rsspage.tmpl` to `atompage.tmpl`.
|
||||||
|
* Verify that [[plugins/more]] plays well with fancy podcasts.
|
||||||
|
* Verify that the feeds validate.
|
||||||
|
* Subscribe to a fancy feed in some common podcatchers and verify
|
||||||
|
display details against a reference podcast.
|
||||||
|
* Verify smooth transitions for two common use cases (see testing
|
||||||
|
details below).
|
||||||
|
* Code review: don't add enclosure divs unless we have enclosures.
|
||||||
|
* Code review: genericize download link for more use cases.
|
||||||
|
* Code review: don't confuse old readers with Atom names in RSS.
|
||||||
|
* Code review: instead of hacking back to `$link`, just provide it.
|
||||||
|
* Code review: show author in addition to feedname, if different.
|
||||||
|
|
||||||
|
### Code review
|
||||||
|
|
||||||
|
+ # XXX better way to compute relative to srcdir?
|
||||||
|
+ my $file = $absurl;
|
||||||
|
+ $file =~ s|^$config{url}/||;
|
||||||
|
|
||||||
|
I don't think ikiwiki offers a better way to do that, because there is
|
||||||
|
normally no reason to do that. Why does it need an url of this form here?
|
||||||
|
--[[Joey]]
|
||||||
|
|
||||||
|
> In all the popular, production-quality podcast feeds I've looked
|
||||||
|
> at, enclosure URLs are always absolute (even when they could be
|
||||||
|
> expressed concisely as relative). [Apple's
|
||||||
|
> example](http://www.apple.com/itunes/podcasts/specs.html#example)
|
||||||
|
> does too. So I told \[[!meta]] to call `urlto()` with the third
|
||||||
|
> parameter true, which means the \[[!inline]] code here gets an
|
||||||
|
> absolute URL in `$pagestate{$p}{meta}{enclosure}`. To compute the
|
||||||
|
> enclosure's metadata, though, we of course need it as a local path.
|
||||||
|
> I didn't see a less
|
||||||
|
> [ongepotchket](http://www.jewish-languages.org/jewish-english-lexicon/words/1402)
|
||||||
|
> way at the time. If you have a better idea, I'm happy to hear it;
|
||||||
|
> if not, I'll add an explanatory comment. --[[schmonz]]
|
||||||
|
|
||||||
|
>> I would be more comfortable with this if two two different forms of url
|
||||||
|
>> you need were both generated by calling urlto. It'd be fine to call
|
||||||
|
>> it more than once. --[[Joey]]
|
||||||
|
|
||||||
|
>>> Heh, it was even easier than that! (Hooray for tests.) Done.
|
||||||
|
>>> --[[schmonz]]
|
||||||
|
|
||||||
|
+<TMPL_IF HTML5><section id="inlineenclosure"><TMPL_ELSE><div id="inlineenclosure"></TMPL_IF>
|
||||||
|
+<TMPL_IF ENCLOSURE>
|
||||||
|
|
||||||
|
Can't we avoid adding this div when there's no enclosure? --[[Joey]]
|
||||||
|
|
||||||
|
> Sure, I've moved the `<TMPL_IF ENCLOSURE>` check to outside the
|
||||||
|
> section-and-div block for `{,inline}page.tmpl`. --[[schmonz]]
|
||||||
|
|
||||||
|
+<a href="<TMPL_VAR ENCLOSURE>">Download this episode</a>
|
||||||
|
|
||||||
|
"Download this episode" is pretty specific to particular use cases.
|
||||||
|
Can this be made more generic, perhaps just "Download"? --[[Joey]]
|
||||||
|
|
||||||
|
> Yep, I got a little carried away. Done. --[[schmonz]]
|
||||||
|
|
||||||
|
-<TMPL_IF AUTHOR>
|
||||||
|
- <title><TMPL_VAR AUTHOR ESCAPE=HTML>: <TMPL_VAR TITLE></title>
|
||||||
|
- <dcterms:creator><TMPL_VAR AUTHOR ESCAPE=HTML></dcterms:creator>
|
||||||
|
|
||||||
|
This change removes the author name from the title of the rss feed, which
|
||||||
|
does not seem necessary for fancy podcasts. And it is a change that
|
||||||
|
could negatively impact eg, Planet style aggregators using ikiwiki. --[[Joey]]
|
||||||
|
|
||||||
|
> While comparing how feeds render in podcatchers, I noticed that
|
||||||
|
> RSS and Atom were inconsistent in a couple ways, of which this was
|
||||||
|
> one. The way I noticed it: with RSS, valuable title space was being
|
||||||
|
> spent to display the author. I figured Atom's display was the one
|
||||||
|
> worth matching. You're right, of course, that planets using the
|
||||||
|
> default template and somehow relying on the current author-in-the-title
|
||||||
|
> rendering for RSS feeds (but not Atom feeds!) would be broken by
|
||||||
|
> this change. I'm having trouble imagining exactly what would break,
|
||||||
|
> though, since guids and timestamps are unaffected. Would it suffice
|
||||||
|
> to provide a note in the changelog warning people to be careful
|
||||||
|
> upgrading their planets, and to customize `rssitem.tmpl` if they
|
||||||
|
> really prefer the old behavior (or don't want to take any chances)?
|
||||||
|
> --[[schmonz]]
|
||||||
|
|
||||||
|
>> A specific example I know of is updo.debian.net, when used with
|
||||||
|
>> rss2email. Without the author name there, one cannot see who posted
|
||||||
|
>> an item. It's worth noting that planet.debian.org does the same thing
|
||||||
|
>> with its rss feed. (That's probably what I copied.) Atom feeds may
|
||||||
|
>> not have this problem, don't know. --[[Joey]]
|
||||||
|
|
||||||
|
>>> Okay, that's easy to reproduce. It looks like this _might_ be
|
||||||
|
>>> a simple matter of getting \[[!aggregate]] to populate author in
|
||||||
|
>>> `add_page()`. I'll see what I can figure out. --[[schmonz]]
|
||||||
|
|
||||||
|
>>>> Yep, that was mostly it. If the feed entry defines an author,
|
||||||
|
>>>> and the author is distinct from the feed name, we now show `NAME:
|
||||||
|
>>>> AUTHOR`, else just show `NAME` (same as always). In addition,
|
||||||
|
>>>> the W3 feed validator says `<dcterms:creator>` is invalid, so
|
||||||
|
>>>> I replaced it with `<dc:creator>`, and all of a sudden `r2e`
|
||||||
|
>>>> gives me better `From:` headers. With the latest on my branch,
|
||||||
|
>>>> when I generate the same planet as updo and run `r2e` over it,
|
||||||
|
>>>> the names I get in `From:` look like so:
|
||||||
|
|
||||||
|
* `"updo: Junio C Hamano"`
|
||||||
|
* `"updo: Greg Kroah-Hartman"`
|
||||||
|
* `"updo: Eric Raymond: esr"` (article author != feed name, so we get both)
|
||||||
|
* `"updo: Jannis Pohlman: Jannis Pohlmann"` (oops! I tweaked the real updo)
|
||||||
|
|
||||||
|
>>>> --[[schmonz]]
|
||||||
|
|
||||||
|
+++ b/templates/rsspage.tmpl
|
||||||
|
+ xmlns:atom="http://www.w3.org/2005/Atom"
|
||||||
|
+<atom:link href="<TMPL_VAR FEEDURL>" rel="self" type="application/rss+xml" />
|
||||||
|
|
||||||
|
Why is it using atom namespace inside an rss feed? What are the chances
|
||||||
|
every crummy rss reader on earth is going to understand this? I'd put it at
|
||||||
|
about 0%; I doubt ikiwiki's own rss reader understands such a mashup.
|
||||||
|
--[[Joey]]
|
||||||
|
|
||||||
|
> The validator I used (<http://validator.w3.org/feed/>) told me to.
|
||||||
|
> Pretty sure it doesn't make anything work better in the podcatchers
|
||||||
|
> I tried. Hadn't considered that it might break some readers.
|
||||||
|
> Removed. --[[schmonz]]
|
||||||
|
|
||||||
|
+<generator>ikiwiki</generator>
|
||||||
|
|
||||||
|
Does this added tag provide any benefits? --[[Joey]]
|
||||||
|
|
||||||
|
> Consistency with the Atom feed, and of course it trumpets ikiwiki
|
||||||
|
> to software and/or curious humans who inspect their feeds. The tag
|
||||||
|
> arrived only in RSS 2.0, but that's already the version we're
|
||||||
|
> claiming to be, and it's over a decade old. Seems much less risky
|
||||||
|
> than the atom namespace bits. --[[schmonz]]
|
||||||
|
|
||||||
|
>> Sounds ok then. --[[Joey]]
|
||||||
Loading…
Reference in New Issue