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

Make feed limit configurable #193

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@DirtyF
Copy link
Member

DirtyF commented Oct 8, 2017

fix #192 #56

@DirtyF DirtyF requested review from pathawks and benbalter Oct 8, 2017

@DirtyF DirtyF force-pushed the feed-limit branch from e9f9358 to edd4fef Oct 8, 2017

@DirtyF DirtyF added the enhancement label Oct 8, 2017

@pathawks

This comment has been minimized.

Copy link
Member

pathawks commented Oct 8, 2017

Rather than changing the limit, I’d rather revisit trying to include the union of most recent 10 posts and posts from the last 48 hours.

I worry that one more config option without an obvious best setting is problematic.

@DirtyF

This comment has been minimized.

Copy link
Member

DirtyF commented Oct 8, 2017

@pathawks That's an interesting alternative approach that could feel like magic even if that behavior makes sense for websites who publish a lot of content. Where do you get the 48h limit from? Is that a good practice?

I went the (simple) explicit way and let people decide what they wanna do.
Most users shouldn't have to change the default anyway, we can all agree to that.

@pathawks

This comment has been minimized.

Copy link
Member

pathawks commented Oct 8, 2017

Where do you get the 48h limit from? Is that a good practice?

I can’t remember where that number came from, but the argument to increase the limit comes from the problem of a site publishing new items faster than consumers are checking for updates. A limit of 48 hours ensures that consumers will get all new items, enen if they only poll every other day.

@pathawks

This comment has been minimized.

Copy link
Member

pathawks commented Oct 8, 2017

Most users shouldn't have to change the default anyway, we can all agree to that.

Yes.

I would add that most users should never think about the contents of their feed XML file. If this plugin does its job, things should Just Work.

DirtyF added some commits Nov 5, 2017

@kylebarbour

This comment has been minimized.

Copy link
Contributor

kylebarbour commented Jan 17, 2018

I'd like to be able to do this as well — is there a reason that having a default of 10 and the possibility to configure further if you wish would be harmful? That would maintain backwards compatibility and extend the usefulness of this plugin to more users.

The union of the last 48 hours and 10 posts doesn't work for me as I update with long delays with interspersed frequent updates, so that would either create an empty feed (when I don't post for more than 48 hours) or the possibility infrequent pollers of missing updates during the high-intensity periods. Being able to set an arbitrary limit would be helpful for that situation and mean that I no longer have to think about it.

@pathawks

This comment has been minimized.

Copy link
Member

pathawks commented Jan 17, 2018

The union of the last 48 hours and 10 posts doesn't work for me as I update with long delays with interspersed frequent updates, so that would either create an empty feed (when I don't post for more than 48 hours) or the possibility infrequent pollers of missing updates during the high-intensity periods.

As long as the feed is polled every 2 days, no updates would be missed.

@pathawks
Copy link
Member

pathawks left a comment

I'm still 👎 on the additional complexity this adds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment