Skip to content

component control switches#1214

Merged
PeterSurda merged 1 commit intoBitmessage:v0.6from
f97ada87:opmode-switches
Apr 11, 2018
Merged

component control switches#1214
PeterSurda merged 1 commit intoBitmessage:v0.6from
f97ada87:opmode-switches

Conversation

@f97ada87
Copy link
Copy Markdown
Contributor

@f97ada87 f97ada87 commented Apr 9, 2018

This PR implements the component control switches discussed in the comments of PR #1149

# queue which will never be handled by a UI. We should clear it to
# save memory.
if shared.thisapp.daemon:
if shared.thisapp.daemon or not state.enableGUI: # FIXME redundant?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, maybe just if not state.enableGUI:

Comment thread src/helper_generic.py
return
logger.error("Got signal %i", signal)
if shared.thisapp.daemon:
if shared.thisapp.daemon or not state.enableGUI: # FIXME redundant?
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure if this needed

Copy link
Copy Markdown
Collaborator

@g1itch g1itch left a comment

Choose a reason for hiding this comment

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

Seems to be acceptable

@f97ada87
Copy link
Copy Markdown
Contributor Author

f97ada87 commented Apr 11, 2018

Thanks for the comments.

There's an unlikely corner case that I can think of, and for this reason I left the redundancy in place. If shared.thisapp.daemon is changed during runtime, after the initialization code in bitmessagemain, the variable state.enableGUI will not magically auto-update, resulting in inconsistent behaviour.
I couldn't find any such cases in the current code, however, I didn't check every single PR in the queue so I can't be 100% sure. I left those overlaps in the code (with FIXME notices) to cover this small uncertainty window, and they can be cleaned up in a subsequent PR once the dust settles. They are harmless in any case.

If the consensus is that they're unnecessary clutter, I can clean them up within this PR.

@PeterSurda
Copy link
Copy Markdown
Member

I'm really happy that there are now constructive debates and development is steadily progressing. This hasn't been the case for many years. Let me know how I can make it easier for you to contribute.

I haven't had that much time for development in the last two months or so, as I'm focusing on having a solid and secure infrastructure. That may take another month or so, then I'll do more development again.

@PeterSurda
Copy link
Copy Markdown
Member

Regarding shared.thisapp.daemon vs. state.enableGUI, I think we'll leave it as it is, including FIXMEs. Later this can be revisited and one of these two options removed, or, if new requirements appear, they could stay.

@PeterSurda PeterSurda merged commit cb59b8a into Bitmessage:v0.6 Apr 11, 2018
@f97ada87 f97ada87 deleted the opmode-switches branch April 11, 2018 15:08
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.

3 participants