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

Correct - Extend msys search to all possible drives #1359

Merged
merged 2 commits into from
Jun 26, 2024
Merged

Conversation

raikoug
Copy link
Contributor

@raikoug raikoug commented Jun 8, 2024

Corrected version - Previously, the code was only checking for the msys installation in the C drive. This commit extends the search to all possible drives on the system. It generates all possible paths by combining each drive letter with each possible path, and then checks each one. This ensures that the msys installation can be found regardless of the drive it's installed on. I proposed a change for a mid file by mistake, this one is working fine with a different drive letter.

Corrected version - Previously, the code was only checking for the msys installation in the C drive. This commit extends the search to all possible drives on the system. It generates all possible paths by combining each drive letter with each possible path, and then checks each one. This ensures that the msys installation can be found regardless of the drive it's installed on.
I proposed a change for a mid file by mistake, this one is working fine with a different drive letter.
gvsbuild/utils/builder.py Outdated Show resolved Hide resolved
@danyeaw
Copy link
Member

danyeaw commented Jun 16, 2024

@raikoug, thanks again for the contribution! Are you still interested in working on this to fix the lint errors and update the solution?

@raikoug
Copy link
Contributor Author

raikoug commented Jun 17, 2024

@raikoug, thanks again for the contribution! Are you still interested in working on this to fix the lint errors and update the solution?

I never played with lint. I will give a look if I can understand something 😸

@danyeaw
Copy link
Member

danyeaw commented Jun 17, 2024

Hi @raikoug, ya, we should probably add a CONTRIBUTING.md file with some of this explained.

poetry run pre-commit install
poetry run pre-commit run --all-files
git commit -m "Fix lint issue"
git push

From then on, all commits will automatically check for lint issues.

@raikoug
Copy link
Contributor Author

raikoug commented Jun 17, 2024

I'm sure that a more experienced team worker would know this.
I cloned the repository, fixed my issue offline, edited the files on browser, and always from browser made the merge req.
I will try to be more git oriented instead of browser :D

@danyeaw
Copy link
Member

danyeaw commented Jun 18, 2024

Hey @raikoug, no problem at all and thanks once again for the contribution. I pushed a commit that cleaned up the formatting and reduced the search to a few drive letters.

@danyeaw danyeaw requested a review from pbor June 18, 2024 02:07
@nacho
Copy link
Contributor

nacho commented Jun 26, 2024

Looks good to me. Thanks

@nacho nacho merged commit af5a653 into wingtk:main Jun 26, 2024
12 checks passed
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.

None yet

3 participants