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

Version log and (limited) notifications log #10178

Merged
merged 2 commits into from Mar 5, 2023
Merged

Version log and (limited) notifications log #10178

merged 2 commits into from Mar 5, 2023

Conversation

zwim
Copy link
Contributor

@zwim zwim commented Mar 3, 2023

This is the successor to #10160

This change is Reviewable

@zwim
Copy link
Contributor Author

zwim commented Mar 3, 2023

This PR will close #1257 and #3418

@zwim zwim marked this pull request as ready for review March 3, 2023 20:35
@Frenzie Frenzie added this to the 2023.03 milestone Mar 3, 2023
@@ -76,5 +78,28 @@ This allows selecting which to show or hide.]]),
end,
separator = true,
},
{
text = _("Show past notifications"),
help_text = _("Show the text of past Notifications."),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
help_text = _("Show the text of past Notifications."),
help_text = _("Show the text of past notifications."),

Seems a redundant though? I'd basically the same.

Copy link
Contributor Author

@zwim zwim Mar 3, 2023

Choose a reason for hiding this comment

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

I will remove the help text.

reader.lua Outdated Show resolved Hide resolved
frontend/ui/elements/screen_notification_menu_table.lua Outdated Show resolved Hide resolved
frontend/version.lua Outdated Show resolved Hide resolved
frontend/logfile.lua Outdated Show resolved Hide resolved
Comment on lines 101 to 109

if not last_log_line then -- empty log file
return ""
end
else -- log file does not exist or can not be opened
return ""
end

return last_log_line
Copy link
Contributor

Choose a reason for hiding this comment

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

Can probably be simplified to just:
return last_log_line or ""

@poire-z poire-z changed the title Version log and (limited) infomessage log Version log and (limited) notifications log Mar 5, 2023
Copy link
Member

@NiLuJe NiLuJe left a comment

Choose a reason for hiding this comment

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

Reviewed 5 of 6 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Frenzie and @zwim)

@Frenzie Frenzie merged commit efd2df6 into koreader:master Mar 5, 2023
2 checks passed
@zwim zwim deleted the VersionLog branch March 5, 2023 14:24
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.

None yet

4 participants