Skip to content
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

Only include docs from the last two days in news sitemap #71

Merged
merged 3 commits into from
Mar 9, 2018

Conversation

cperryk
Copy link
Contributor

@cperryk cperryk commented Mar 9, 2018

The news sitemap should only include articles from the last two days as per Google's guide on news sitemap creation. This PR modifies amphora's search query to the news-sitemap-entries index so that it only gets articles from the last two days.

Copy link
Contributor

@reubenson reubenson left a comment

Choose a reason for hiding this comment

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

lgtm, but question around date logic

}, {
range: {
publication_date: {
gte : 'now-2d/d',
Copy link
Contributor

@reubenson reubenson Mar 9, 2018

Choose a reason for hiding this comment

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

how strict is google on 'two days'? /d rounds down to the nearest day, so a query that's executed at 23:59 would grab articles from the past ~72 hours. should the query instead be for now-48h?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My thought is that if Google meant "the last 48 hours" the guide would more precisely say "the last 48 hours." But it's ambiguous. I'll switch to the 48-hour window b/c that's what the Washington Post appears to be using for their news sitemaps.

@coveralls
Copy link

coveralls commented Mar 9, 2018

Coverage Status

Coverage increased (+6.0e-05%) to 99.863% when pulling f18a668 on news-sitemap-fix into c54a604 on master.

@cperryk cperryk merged commit 63added into master Mar 9, 2018
@cperryk cperryk deleted the news-sitemap-fix branch March 9, 2018 17:18
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.

3 participants