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

Partial fix of #540: do not emit others object signals. #656

Merged
merged 1 commit into from
Jul 8, 2021
Merged

Partial fix of #540: do not emit others object signals. #656

merged 1 commit into from
Jul 8, 2021

Conversation

asashnov
Copy link
Contributor

@asashnov asashnov commented Jun 16, 2021 •
edited
Loading

This pull request fixes the codebase: signals should be emitted only by the object itself.

This is the "part 1" PR for #540:

  • stop emitting TabBar::currentTitleChanged() from class ZimView
  • stop emitting TabBar::currentZimIdChanged() from class ZimView - removed, not used anywhere

@asashnov asashnov force-pushed the 540-part1-title-icon-changed branch 2 times, most recently from 1828732 to 3c52284 Compare June 16, 2021 19:03
@asashnov asashnov requested a review from mgautierfr June 16, 2021 19:06
@mgautierfr mgautierfr mentioned this pull request Jun 24, 2021
Closed
@mgautierfr
Copy link
Member

The code seems good to me.
But a bug in libkiwix prevent the packages to be build (kiwix/libkiwix#564).
We could merge this PR right now, but as there is no urgence of this PR, I would prefer to wait we fix the issue in libkiwix and be sure we can compile correctly here.

@stale
Copy link

stale bot commented Jul 1, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be now be reviewed manually. Thank you for your contributions.

@stale stale bot added the stale label Jul 1, 2021
@asashnov @kelson42
Partial fix of #540: do not emit others object signals.
d8e585d 
Signal currentZimIdChanged() isn't used anywhere, removed from code.
@kelson42 kelson42 force-pushed the 540-part1-title-icon-changed branch from 3c52284 to d8e585d Compare July 8, 2021 11:15
@stale stale bot removed the stale label Jul 8, 2021
@kelson42 kelson42 merged commit 7fe5134 into master Jul 8, 2021
@kelson42 kelson42 deleted the 540-part1-title-icon-changed branch July 8, 2021 11:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgautierfr mgautierfr Awaiting requested review from mgautierfr

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@asashnov @mgautierfr @kelson42