-
Notifications
You must be signed in to change notification settings - Fork 296
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
Refactor RSS feeds generation, and do it through templates #515
Conversation
Nice! Markdown plugin will be able to generate formatted feeds soon 😄 (cfr #485) |
class FeedBuilder | ||
{ | ||
/** | ||
* @var string Constant: RSS feed type. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the trailing dots at the end of each comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Habits from my pro Sonar rules.
Very cool work @ArthurHoaro ! It's nice to see chunks of code drifting away from |
Thanks for the review! I'll work on it soon. |
ATOM feed improvement: * Adds a subtitle to match RSS feed behavior. * Better syntax for categories (see http://edward.oconnor.cx/2007/02/representing-tags-in-atom ). * Use locale to set the language
Improvements: * Add searchtags in categories domain URL. * Language is now based on the locale. * Add a generator tag. * self link is always displayed.
Minor changes: * Fix the date which was in a invalid format. * Avoid empty categories (tags). * Use the locale to set the language
…S feed. Create an example of the new hook in the demo plugin.
f3b3265
to
26c7b67
Compare
I've update the commit log to take your review into account. I've also added a new commit: 26c7b67
It's not really related to this PR, but it has to be done, and lighten the |
26c7b67
to
fe2f613
Compare
* | ||
* @return array Link array with feed attributes. | ||
*/ | ||
public function buildItem($link, $pageaddr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unless directly tested, can be either private
or protected
Thanks 👍 |
Tested, works fine ;-) |
* search type now carried by LinkDB in order to factorize code between different search sources. * LinkDB->filter split in 3 method: filterSearch, filterHash, filterDay (we know what type of filter is needed). * filterHash now throw a LinkNotFoundException if it doesn't exist: internal implementation choice, still displays a 404. * Smallhash regex has been rewritten. * Unit tests update
fe2f613
to
528a6f8
Compare
Thanks again! |
This PR is a bit huge, please look at the commit log to review it. I've tried to split everything in different commits to make it understandable.
To sum it up:
ENABLE_RSS_PERMALINKS
setting in administration has been reworded.Fixes #273
Note that this doesn't concern Daily RSS feed.
PS: tig is awesome.