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

BF/RF: Application logging overhaul #4750

Merged
merged 6 commits into from
Apr 8, 2022

Conversation

mdcutone
Copy link
Member

@mdcutone mdcutone commented Apr 4, 2022

This PR removes previous logging 'blind-spots' in the application. Now errors that occur during GUI initialization are captured a printed to 'last_app_load.log'. Also, messages written to stdout/sterr from sub-processes also show up in that log.

I also made changes to how subprocess pipes are handled to be less implicit about their state when shutting down (better communication between main and sub-threads. This removes the 'hang' that can occur when closing down a subprocess.

@mdcutone mdcutone requested a review from peircej April 4, 2022 17:37
@lgtm-com
Copy link

lgtm-com bot commented Apr 4, 2022

This pull request introduces 3 alerts and fixes 1 when merging 21948bb into 2a2f56d - view on LGTM.com

new alerts:

  • 3 for Unused import

fixed alerts:

  • 1 for Use of the return value of a procedure

… correct time, error is raised otherwise.
@lgtm-com
Copy link

lgtm-com bot commented Apr 4, 2022

This pull request introduces 3 alerts and fixes 1 when merging 11a6e15 into 2a2f56d - view on LGTM.com

new alerts:

  • 3 for Unused import

fixed alerts:

  • 1 for Use of the return value of a procedure

@codecov
Copy link

codecov bot commented Apr 7, 2022

Codecov Report

Merging #4750 (a82d320) into release (9fcdb76) will decrease coverage by 0.00%.
The diff coverage is 32.75%.

@@             Coverage Diff             @@
##           release    #4750      +/-   ##
===========================================
- Coverage    45.32%   45.31%   -0.01%     
===========================================
  Files          294      294              
  Lines        58845    58861      +16     
  Branches     10393    10394       +1     
===========================================
+ Hits         26671    26673       +2     
- Misses       29793    29810      +17     
+ Partials      2381     2378       -3     
Impacted Files Coverage Δ
psychopy/app/_psychopyApp.py 46.92% <ø> (+0.04%) ⬆️
psychopy/app/jobs.py 35.46% <0.00%> (-0.36%) ⬇️
psychopy/app/runner/scriptProcess.py 27.00% <0.00%> (+1.03%) ⬆️
psychopy/app/__init__.py 54.16% <36.00%> (+0.11%) ⬆️
psychopy/app/console.py 50.81% <57.14%> (-13.77%) ⬇️
psychopy/app/coder/coder.py 29.20% <100.00%> (+0.04%) ⬆️
psychopy/app/stdOutRich.py 54.65% <0.00%> (-9.31%) ⬇️
psychopy/visual/window.py 62.22% <0.00%> (-0.23%) ⬇️
... and 2 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@lgtm-com
Copy link

lgtm-com bot commented Apr 7, 2022

This pull request introduces 3 alerts and fixes 1 when merging a82d320 into 9fcdb76 - view on LGTM.com

new alerts:

  • 3 for Unused import

fixed alerts:

  • 1 for Use of the return value of a procedure

@TEParsons TEParsons merged commit 5586e60 into psychopy:release Apr 8, 2022
@mdcutone mdcutone deleted the release-logging-fix branch June 23, 2022 16:30
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.

2 participants