-
Notifications
You must be signed in to change notification settings - Fork 685
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
Set published date with hash based timestamp #9219
Conversation
⚡ Deployed with Pantheon Decoupled This build was successfully deployed with Pantheon. You can track the build logs here. 👀 Preview: https://pr-9219-documentation.appa.pantheon.site |
⚡ Deployed with Pantheon Decoupled This build was successfully deployed with Pantheon. You can track the build logs here. 👀 Preview: https://pr-9219-documentation.appa.pantheon.site |
⚡ Deployed with Pantheon Decoupled This build was successfully deployed with Pantheon. You can track the build logs here. 👀 Preview: https://pr-9219-documentation.appa.pantheon.site |
Added multiple test entries in a single day with the same published date and they were all 3 picked up by my RSS reader @mel-miller can you review? |
⚡ Deployed with Pantheon Decoupled This build was successfully deployed with Pantheon. You can track the build logs here. 👀 Preview: https://pr-9219-documentation.appa.pantheon.site |
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.
@rachelwhitton -- TBH, I'm not that familiar with this type of thing. But the solution seems good to me and I don't see anything dangerous introduced.
⚡ Deployed with Pantheon Decoupled This build was successfully deployed with Pantheon. You can track the build logs here. 👀 Preview: https://pr-9219-documentation.appa.pantheon.site |
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.
lgtm
Co-authored-by: Steve Persch <[email protected]>
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.
update is a bit more granular than strictly necessary, but I like the breakdown and I don't think it's a problem. I imagine getting into the weeds discussing the arbitrariness of the timestamp might lead to questions about notes being ordered incorrectly (or call attention to the discrepancy between where release notes appear in readers vs on the website).
⚡ Deployed with Pantheon Decoupled This build was successfully deployed with Pantheon. You can track the build logs here. 👀 Preview: https://pr-9219-documentation.appa.pantheon.site |
⚡ Deployed with Pantheon Decoupled This build was successfully deployed with Pantheon. You can track the build logs here. 👀 Preview: https://pr-9219-documentation.appa.pantheon.site |
This reverts commit 4a8ffb5.
⚡ Deployed with Pantheon Decoupled This build was successfully deployed with Pantheon. You can track the build logs here. 👀 Preview: https://pr-9219-documentation.appa.pantheon.site |
Closes #9126
Fixes known RSS bug by setting an arbitrary hour, minute, and second for each release note within the RSS feed by deriving a hash of the note.
Adds new release note: Bug fix for Pantheon release notes RSS feed