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.

Sign up for GitHub

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

Jump to bottom

Bold the verse of the day in reader view #19

Merged
merged 5 commits into from
Jul 5, 2024
Merged

Bold the verse of the day in reader view #19

merged 5 commits into from
Jul 5, 2024

Conversation

tj330
Copy link
Contributor

@tj330 tj330 commented Jul 2, 2024 •
edited by iwoithe
Loading

Resolves: #5

@tj330 tj330 marked this pull request as ready for review July 2, 2024 03:56
@Correct-Syntax Correct-Syntax marked this pull request as draft July 2, 2024 04:02
@iwoithe iwoithe changed the title Modify notifications.py Bold the verse of the day in reader view Jul 2, 2024
Correct-Syntax
Correct-Syntax requested changes Jul 2, 2024
View reviewed changes
Copy link
Member

@Correct-Syntax Correct-Syntax left a comment

Choose a reason for hiding this comment

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

Initial review

src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
Correct-Syntax
Correct-Syntax requested changes Jul 3, 2024
View reviewed changes
Copy link
Member

@Correct-Syntax Correct-Syntax left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Great job @tj330. Just a few more fixes. Once these are done, then I'll let @iwoithe review to see if he has any further comments.

src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
@Correct-Syntax Correct-Syntax requested a review from iwoithe July 3, 2024 15:44
@Correct-Syntax Correct-Syntax marked this pull request as ready for review July 3, 2024 15:44
@Correct-Syntax
Copy link
Member

FYI @iwoithe I found a few bugs during testing of this PR. They are out-of-scope for this PR, so I'll open a new PR to address them once this one is merged.

iwoithe
iwoithe requested changes Jul 5, 2024
View reviewed changes
Copy link
Member

@iwoithe iwoithe left a comment

Choose a reason for hiding this comment

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

Looking good, just a few last (minor) things to change!

Thanks!

src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
src/biblenotify/core/notifications.py Outdated Show resolved Hide resolved
iwoithe
iwoithe approved these changes Jul 5, 2024
View reviewed changes
Copy link
Member

@iwoithe iwoithe left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks again 😊

@iwoithe iwoithe merged commit e1018f3 into BibleNotify:master Jul 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iwoithe iwoithe iwoithe approved these changes

@Correct-Syntax Correct-Syntax Correct-Syntax approved these changes

Assignees

@tj330 tj330

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Highlight verse in Reader
3 participants
@tj330 @Correct-Syntax @iwoithe