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

fix(greader): set CrawlTimeMsec at the correct precision #2670

Closed
wants to merge 1 commit into from

Conversation

mbestavros
Copy link

@mbestavros mbestavros commented May 28, 2024

Do you follow the guidelines?

This PR is a quick and simple fix for #2669, where you can find a full writeup. Resolves #2669.

@@ -1022,7 +1022,7 @@ func (h *handler) streamItemContentsHandler(w http.ResponseWriter, r *http.Reque
Title: entry.Title,
Author: entry.Author,
TimestampUsec: fmt.Sprintf("%d", entry.Date.UnixNano()/(int64(time.Microsecond)/int64(time.Nanosecond))),
CrawlTimeMsec: fmt.Sprintf("%d", entry.Date.UnixNano()/(int64(time.Microsecond)/int64(time.Nanosecond))),
CrawlTimeMsec: fmt.Sprintf("%d", entry.Date.UnixNano()/(int64(time.Millisecond)/int64(time.Nanosecond))),
Published: entry.Date.Unix(),
Updated: entry.Date.Unix(),
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering why it was implemented this way in the first place. It could probably be changed like this but I need to test first:

-                       TimestampUsec: fmt.Sprintf("%d", entry.Date.UnixNano()/(int64(time.Microsecond)/int64(time.Nanosecond))),
-                       CrawlTimeMsec: fmt.Sprintf("%d", entry.Date.UnixNano()/(int64(time.Microsecond)/int64(time.Nanosecond))),
+                       TimestampUsec: fmt.Sprintf("%d", entry.Date.UnixMicro()),
+                       CrawlTimeMsec: fmt.Sprintf("%d", entry.Date.UnixMilli()),
                        Published:     entry.Date.Unix(),
-                       Updated:       entry.Date.Unix(),
+                       Updated:       entry.ChangedAt.Unix(),

Copy link
Member

Choose a reason for hiding this comment

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

I have submitted PR #2673 with the diff mentioned above. I have tested with Reeder on iOS.

Copy link
Author

@mbestavros mbestavros May 31, 2024

Choose a reason for hiding this comment

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

@fguillot Thanks for the quick fix! I'll check how it works on Read You once it rolls out to the hosted service and try to remember to post an update here.

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

Successfully merging this pull request may close these issues.

greader: crawlTimeMsec is sent at microsecond precision, instead of conventional millisecond precision
2 participants