Skip to content

Log NVDA version number at the top of the log file to make debugging easier#10004

Merged
michaelDCurran merged 8 commits into
nvaccess:masterfrom
lukaszgo1:I9803
Sep 16, 2019
Merged

Log NVDA version number at the top of the log file to make debugging easier#10004
michaelDCurran merged 8 commits into
nvaccess:masterfrom
lukaszgo1:I9803

Conversation

@lukaszgo1

@lukaszgo1 lukaszgo1 commented Jul 31, 2019

Copy link
Copy Markdown
Contributor

Link to issue number:

Closes #9803

Summary of the issue:

When testing a issue with various version of NVDA is is useful to have version number easily available in the log file. At the moment it isn't at the top and if log level is set to disabled is not logged at all.

Description of how this pull request fixes the issue:

Version number is moved to the second line of the log and buildVersion is used to retrieve it instead of versionInfo.

Testing performed:

  1. Started NVDA from source with logging level set to debug and info - ensured that build number is logged in the expected place.
  2. Started NVDA with log level set to disabled closed it and inspected the log - the build number is also logged as expected.

Known issues with pull request:

None

Change log entry:

Section: Changes

The NVDA version number is now logged as the first message in the log. Note that it happens also when logging is disabled from the GUI

@AppVeyorBot

Copy link
Copy Markdown

PR introduces Flake8 errors 😲

See test results for Failed build of commit ca793c64d3

Comment thread source/core.py
@@ -1,9 +1,9 @@
# -*- coding: UTF-8 -*-
#core.py

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.

Please restore this line

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@LeonarddeR According to the copyright headers wiki page

Note: We decided to remove the filename comment from the top of the file, since it doesn't really add anything, and is a source of error on file rename / copying copyright headers between files.

If we arre no longer adding it to the new files why shouldn't we remove it from the existing ones during updating of the copyright headers?

@LeonarddeR

LeonarddeR commented Aug 12, 2019 via email

Copy link
Copy Markdown
Collaborator

@michaelDCurran michaelDCurran left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If NVDA is started with --log-level 30 or higher, the NVDA version is still not logged. I think that nvda.pyw should initially force the log level to info, log the version, and then set the log level based on the commandline.

@lukaszgo1

Copy link
Copy Markdown
Contributor Author

@michaelDCurran I'm afraid I have to disagree here.
The main reason for creating this pr was the fact that when log is initially set to disabled, and then to something higher version of NVDA is never logged. This is problematic when attempting to identify a bug which occurred during particular NVDA run. I can imagine situation in which someone has logging disabled most of the time he/she experienced something unexpected so the log level is raised to be able to gather more details.
However when logging level is specified as command line argument there is no possibility to change logging level, therefore in such situation NVDA needs to be restarted. I don't see a point in logging version number alone when noting else could be logged during particular NVDA session. You can argue that this is inconsistent with what happens when logging is set from GUI, however I consider the fact that anything at all is logged when log level is set to disabled a necessity (we cannot disable logging until log level is read from configuration), and not an desirable behavior.

@michaelDCurran michaelDCurran merged commit 9d04d05 into nvaccess:master Sep 16, 2019
@nvaccessAuto nvaccessAuto added this to the 2019.3 milestone Sep 16, 2019
michaelDCurran added a commit that referenced this pull request Sep 16, 2019
@lukaszgo1 lukaszgo1 deleted the I9803 branch September 23, 2019 12:13
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.

NVDA should log its version number immediately, at all log levels

5 participants