Skip to content

Correct promise order in RSS feed generator#9726

Merged
kirrg001 merged 5 commits intoTryGhost:masterfrom
uqee:rss-promise-order
Sep 10, 2018
Merged

Correct promise order in RSS feed generator#9726
kirrg001 merged 5 commits intoTryGhost:masterfrom
uqee:rss-promise-order

Conversation

@uqee
Copy link
Copy Markdown

@uqee uqee commented Jul 9, 2018

  • the 'rss.feed' filter used to get an empty feed as a parameter, because item generation is done via promises
  • now it waits until all items have been added

no issue

- the 'rss.feed' filter used to get an empty feed as a parameter, because item generation is done via promises
- now it waits until all items have been added
@kirrg001 kirrg001 self-assigned this Jul 10, 2018
filters.doFilter('rss.item', item, post).then(function then(item) {
feed.item(item);
return Promise.all(
data.posts.map(function map(post) {

This comment was marked as abuse.

This comment was marked as abuse.

Copy link
Copy Markdown
Contributor

@kirrg001 kirrg001 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR 👍

I left one comment https://github.com/TryGhost/Ghost/pull/9726/files#r201314618 for discussion :)

no issue

The posts are provided in a specific order, which should be preserved in a corresponding sequence of promises.
@uqee
Copy link
Copy Markdown
Author

uqee commented Jul 10, 2018

Local yarn test succeeds, but Travis has timeouted. I guess I don't have enough rights to restart it.

@kirrg001
Copy link
Copy Markdown
Contributor

I have restarted the build 👍

@uqee
Copy link
Copy Markdown
Author

uqee commented Jul 10, 2018

Awesome! Any more suggestions?

feed.item(item);
return data.posts.reduce(function (feedPromise, post) {
return feedPromise.then(function () {
var item = generateItem(post, siteUrl, data.secure);

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

return feed.item(item);
});
});
}, Promise.resolve()).then(function () {

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

filters.doFilter('rss.item', item, post).then(function then(item) {
feed.item(item);
});
const {urlFor} = urlService.utils;

This comment was marked as abuse.

This comment was marked as abuse.

@uqee
Copy link
Copy Markdown
Author

uqee commented Jul 18, 2018

Restart it, please.

Copy link
Copy Markdown
Contributor

@kirrg001 kirrg001 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kirrg001 kirrg001 merged commit f12fd4b into TryGhost:master Sep 10, 2018
rshbhgrg pushed a commit to rshbhgrg/Ghost that referenced this pull request Sep 11, 2018
no issue

- the 'rss.feed' filter used to get an empty feed as a parameter, because item generation is done via promises
- now it waits until all items have been added
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