Skip to content

Fix Windows path compatibility in cloud storage URLs#786

Merged
jonbrenas merged 4 commits intomasterfrom
fix-windows-path-mainrepo
May 30, 2025
Merged

Fix Windows path compatibility in cloud storage URLs#786
jonbrenas merged 4 commits intomasterfrom
fix-windows-path-mainrepo

Conversation

@mohamed-laarej
Copy link
Copy Markdown
Collaborator

@mohamed-laarej mohamed-laarej commented May 28, 2025

Re: issue #783
This PR addresses the windows path compatibility. Supersedes PR #784 from my fork to resolve CI test failures.

@mohamed-laarej
Copy link
Copy Markdown
Collaborator Author

Hi @jonbrenas ,
I've opened this new PR from a branch on the main repository to resolve the CI test failures we encountered with the earlier PR from my fork. In addition to migrating the branch, I've made several adjustments to fix MyPy type errors that were causing issues during the CI checks.
Please note that the MyPy errors in the CI weren't visible in my local environment, which is why I needed to commit the fixes iteratively to verify their effect in the CI pipeline.

This PR supersedes PR #784 .
Could you please take a look when you have a moment? Thanks!

Copy link
Copy Markdown
Collaborator

@jonbrenas jonbrenas left a comment

Choose a reason for hiding this comment

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

Thanks @mohamed-laarej. This looks correct and the mypy fixes (while slightly annoying and not always helping make the code clearer for humans) are probably a good thing.

@jonbrenas jonbrenas merged commit a39a3c1 into master May 30, 2025
10 checks passed
@mohamed-laarej
Copy link
Copy Markdown
Collaborator Author

Hi @jonbrenas ,
Thank you very much for reviewing and merging my first piece of code in an open source project! I really appreciate your guidance and feedback throughout the process. It means a lot to me as I continue learning and contributing. Looking forward to working on more improvements with your support!

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