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

SG-36346 Dynamic hard drive lookup in windows #204

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

carlos-villavicencio-adsk
Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk commented Dec 4, 2024

Description

Introducing win32api.GetLogicalDriveStrings() to get the list of available drive letters in Windows.

In terms of performance, win32api.GetLogicalDriveStrings() is generally very fast and efficient. It directly interfaces with the Windows API to get the list of logical drives, which is a relatively quick operation.

Regarding the glob search, I've done lookup tests with a few different drive letters and it works with no major difference in performance.

Testing

Install Alias software in a different drive than C:. SGD should be able to locate the software and add its icon normally.

Copy link
Contributor

@staceyoue staceyoue left a comment

Choose a reason for hiding this comment

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

LGTM!

For testing, please ask @robomobo to take a look

startup.py Outdated Show resolved Hide resolved
startup.py Show resolved Hide resolved
startup.py Outdated
# of the supported operating systems. The templates are used for both
# globbing and regex matches by replacing the named format placeholders
# with an appropriate glob or regex string.
self.EXECUTABLE_TEMPLATES = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a big fan of overwritting the EXECUTABLE_TEMPLATES variable. Upper case variables are constants by conventions.

Could you instead create a new local variable and make sure it's used in the rest of the function?

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.

4 participants