Skip to content

Conversation

@yanick
Copy link
Contributor

@yanick yanick commented Apr 12, 2019

Add a sort option to sort the entries

Description

Right now there is no sorting done on the feed entries. For feeds that have more entries than
the count option allows, one might want to get the latest entries, or the most popular, etc. Hence the need to be able to sort the entries.

@webmasterish
Copy link
Owner

Definitely!

One minor thing that has to do with my unjustifiable OCD; please use hard tabs of 2 spaces in the following:

// optional sorting function for the entries.
// Gets the array entries as the input, expects the sorted array
// as its output.
// e.g.: sort: entries => _.reverse( _.sortBy( entries, 'date' ) ),
sort: entries => entries, // defaults to just returning it as it is

const pages = this.options.sort(this.pages).slice( 0, this.options.count );

Thanks @yanick!

@yanick
Copy link
Contributor Author

yanick commented Apr 14, 2019

please use hard tabs of 2 spaces in the following

Barbarian. ;-) But your house, your rules. Spaces are tabified.

And I didn't know if you wanted to make sorting by reverse date the default. On one hand, it makes sense, on the other, it's a change in default behavior. So I kept the PR minimal, leaving you with the liberty to do what you'd prefer.

Enjoy! :-)

@webmasterish
Copy link
Owner

Barbarian. ;-) But your house, your rules. Spaces are tabified

Tell me about it! I have to live with that!

I like it minimal just the way you did it, this way it defaults to however the pages are passed in extendPageData, which currently, if I'm not mistaken, default to descending by date.

Thanks again @yanick!

@webmasterish webmasterish merged commit 0b7035e into webmasterish:master Apr 14, 2019
@yanick
Copy link
Contributor Author

yanick commented Apr 14, 2019

I like it minimal just the way you did it, this way it defaults to however the pages are passed in extendPageData, which currently, if I'm not mistaken, default to descending by date.

Either desc by date or random. I just saw that it didn't do exactly what I did, so I dove in. ^.^

Thanks for the quick response, and thanks for an awesome plugin. This makes feed generation as painless as it can be, which is 💯.

@webmasterish
Copy link
Owner

Thanks you @yanick! You made my day!

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