Add NVDA estimated size to add/remove programs #13909
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See test results for failed build of commit a7c41018d6 |
See test results for failed build of commit a5b7c14a78 |
seanbudd
reviewed
Jul 18, 2022
| """Calculates the size of a directory in bytes. | ||
| """ | ||
| total = 0 | ||
| with os.scandir(path) as iterator: |
There was a problem hiding this comment.
Rather than using recursion on getDirectorySize, it might be better to use glob which returns files within folders recursively.
There was a problem hiding this comment.
Thanks for the suggestion. I just tried glob, but there are two disadvantages:
- Glob simply returns files and folders, so we need to call os.path.isfile on every node
- It is noticeably slower
Co-authored-by: Sean Budd <seanbudd123@gmail.com>
LeonarddeR
force-pushed
the
estimatedSize
branch
from
664910d
to
252890f
Compare
seanbudd
approved these changes
Jul 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link to issue number:
None
Summary of the issue:
In Windows Add/remove programs, there is a column that displays the estimated installation size of a product. This column is empty for NVDA.
Description of user facing changes
Calculate install size of the NVDA program files folder, and add it to the registry. This will be visible in Add/remove programs.
Description of development approach
I decided to calculate estimated size during installation. This takes the SystemConfig into account as that is part of the program files directory. This means that the estimated size is also based on the system config. I think this is OK, because when removing NVDA from Add/remove programs, the systemConfig dir is removed as well. Long story short, the estimated size is the best estimate we could get at time of installation
Testing strategy:
installed the build from appveyor. Verified that the estimated size is visible in add/remove programs and equal to the size of C:\Program Files (x86)\NVDA
Known issues with pull request:
None known
Change log entries:
Changes
Code Review Checklist: