Skip to content

Conversation

@shizunge
Copy link
Contributor

@shizunge shizunge commented Jan 6, 2021

In the edit feed page we have already shown the "last check". Now add next_check_at as well.
This does not complicate the UI, nevertheless this still provides some information to users if they want to debug problems.
issue #699 asks to display the value in the "feeds" page. But I think there is too much information on that page already. So I do not add the value to the "feeds" page.

Copy link
Member

@fguillot fguillot left a comment

Choose a reason for hiding this comment

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

This pull-request contains unrelated commits. Only changes related to displaying next_check_at value should be present here.

@shizunge
Copy link
Contributor Author

shizunge commented Jan 7, 2021

This pull-request contains unrelated commits. Only changes related to displaying next_check_at value should be present here.

It depends on #938. Could you review #938 firstly please?
I usually do not want to create changelist (CL) chains as I do not know if github support them.
But since #938 simplifies feed queries a lot, I do hope this CL depends on it, to avoid resolving conflict when rebase.

I still pushed this CL here to let people know that this feature is in progress and I can collect feedback.

@shizunge shizunge force-pushed the display-next-check-at branch 5 times, most recently from ad015be to da9f1f6 Compare January 18, 2021 21:41
@shizunge shizunge requested a review from fguillot January 18, 2021 21:42
<div class="panel">
<ul>
<li><strong>{{ t "page.edit_feed.last_check" }} </strong><time datetime="{{ isodate .feed.CheckedAt }}" title="{{ isodate .feed.CheckedAt }}">{{ elapsed $.user.Timezone .feed.CheckedAt }}</time></li>
<li><strong>{{ t "page.edit_feed.next_check" }} </strong><time datetime="{{ isodate .feed.NextCheckAt }}" title="{{ isodate .feed.NextCheckAt }}">{{ .feed.NextCheckAt }}</time><br />({{ t "page.edit_feed.next_check_note" }})</li>
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to have this line shown only when the scheduler is configured to entry_frequency. Then the text "page.edit_feed.next_check_note" won't be necessary.

Copy link
Contributor Author

@shizunge shizunge Jan 18, 2021

Choose a reason for hiding this comment

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

I am ok to remove the note. (Instead, adding the notes to FAQ or the help)
But I think the next_check_at will still be helpful even when the scheduler is round robin.
If a user sets the polling interval for a single feed (#949), but uses an incompatible global option, they may not get what they want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we keep the next_check_at for all schedulers, the note is relevant. People may ask: why my feed is not fetched at the next_check_at. The note answers that question.
Let me know if you want to keep the notes or not.
Thanks,
Best,

@shizunge shizunge requested a review from fguillot January 18, 2021 22:53
@shizunge shizunge force-pushed the display-next-check-at branch 4 times, most recently from 30551e0 to 1d8cef5 Compare January 28, 2021 17:17
@shizunge shizunge force-pushed the display-next-check-at branch from 1d8cef5 to af897d1 Compare January 31, 2021 02:47
@shizunge shizunge force-pushed the display-next-check-at branch from af897d1 to e4d7c6c Compare February 15, 2021 05:49
@fguillot
Copy link
Member

fguillot commented Jul 5, 2021

Closing out-of-sync and inactive pull-requests.

Feel free to resubmit this PR as long as you follow the guidelines.

@fguillot fguillot closed this Jul 5, 2021
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.

2 participants