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

Issue 8878 - Global Config app not closing if Help Window is Open #200

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

alimirjamali
Copy link

@alimirjamali alimirjamali commented Jun 19, 2024

Fixes QubesOS/qubes-issues#8878

New Global Config app not closing if disposable in which doc link was opened is still running

Solution: hide the main window while waiting for subprocess threads to finish.

@alimirjamali alimirjamali changed the title Issue 8878 fix Fixes Issue 8878 Jun 19, 2024
@andrewdavidwong
Copy link
Member

@alimirjamali: FYI, if you want GitHub to automatically close the associated issue when this PR is merged, you must use

Fixes QubesOS/qubes-issues#8878

instead of

Fixes for Github Issue QubesOS/qubes-issues#8878

in the PR or commit message.

The intervening for Github Issue apparently causes GitHub to no longer recognize the Fixes keyword.

@alimirjamali
Copy link
Author

@alimirjamali: FYI, if you want GitHub to automatically close the associated issue

You are such a life saver. I was wondering why this does not show up as a linked PR in the Github Issue. And given up last night.

@ben-grande
Copy link

Please reword the commit message QubesOS/qubes-core-agent-linux#505 (comment)

@alimirjamali
Copy link
Author

Please reword the commit message

Done. Thanks

@alimirjamali alimirjamali changed the title Fixes Issue 8878 Issue 8878 - Global Config app not closing if Help Window is Open Jun 21, 2024
self.main_window.hide()
while Gtk.events_pending():
Gtk.main_iteration()
''' Then wait for disposable helper threads to finish '''
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
''' Then wait for disposable helper threads to finish '''
# Then wait for disposable helper threads to finish

Pylint dislikes this sort of comments - the triple-quotes should only be used for docstrings at the start of the method/class/whatnow, otherwise # should be used. (the same comment applies to the previous comment, it wasn't flagged by pylint because it's at the start of the function, so it thought it was a docstring describing the function.

If you care about backstory for this, it's in https://peps.python.org/pep-0257/ :)

Copy link
Author

Choose a reason for hiding this comment

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

If you care about backstory for this, it's in https://peps.python.org/pep-0257/ :)

Thank you very much for the link. I will read it. I just changed the comments to start with # and forced pushed the amended commit.

Copy link

codecov bot commented Jun 21, 2024

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 93.20%. Comparing base (0ec6462) to head (01a495a).

Files Patch % Lines
qubes_config/global_config/global_config.py 0.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #200      +/-   ##
==========================================
- Coverage   93.25%   93.20%   -0.05%     
==========================================
  Files          57       57              
  Lines       10594    10600       +6     
==========================================
+ Hits         9879     9880       +1     
- Misses        715      720       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@marmarta marmarta left a comment

Choose a reason for hiding this comment

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

Codecov is being silly here, this is good.

@marmarek marmarek merged commit 5892add into QubesOS:main Jun 23, 2024
2 of 4 checks passed
@alimirjamali alimirjamali deleted the Issue-8878 branch October 29, 2024 18:32
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.

New Global Config app not closing if disposable in which doc link was opened is still running
5 participants