Skip to content

Implementing URI::smb for Samba/CIFS #149

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

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

Conversation

buralien
Copy link

This simple addition will allow working with smb scheme URIs.

Copy link
Member

@oalders oalders left a comment

Choose a reason for hiding this comment

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

Thanks, @buralien!

@oalders
Copy link
Member

oalders commented Mar 10, 2025

@buralien please see the failing tests.

@haarg
Copy link
Member

haarg commented Mar 10, 2025

The overridden user method no longer allows modifying the user. The new domain method also doesn't allow modifying the domain, as most methods of this type do.

The methods also seem to be unescaping twice, since the parent user method does unescaping.

@buralien
Copy link
Author

I apologize for the lack of focus when writing the code. I have fixed the comments and the failing tests. Thanks for helping me think more :)

@buralien buralien requested a review from oalders March 11, 2025 21:55
@buralien
Copy link
Author

Is there anything I can do to help fix these failing builds? It doesn't seem like anything from my code, but I could be wrong.

@oalders
Copy link
Member

oalders commented Mar 18, 2025

Thanks for the offer. 🙏🏻 You could try disabling fail-fast in .github/workflows/dzil-build-and-test.yml and see what actually breaks. If it's all of the Windows builds, then maybe we need to pin the failing module(s) to earlier versions.

@oalders
Copy link
Member

oalders commented Mar 22, 2025

Looks like we can just exclude 5.16 on Windows from the build matrix.

@oalders oalders requested a review from haarg March 23, 2025 15:06
@oalders
Copy link
Member

oalders commented Apr 7, 2025

@haarg does this look ok now?

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.

3 participants