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

Change mkdirs to static #745

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Change mkdirs to static #745

wants to merge 1 commit into from

Conversation

hma13
Copy link

@hma13 hma13 commented Jan 8, 2023

No description provided.

@hma13 hma13 requested a review from hierynomus as a code owner January 8, 2023 04:35
@ecki
Copy link

ecki commented Jan 8, 2023

The idea is good, but it was already released. Do you want to break the API - in that case maybe a release notes is a good ideal.

also should the class get a private constrictor if all util methods will be static?

btw the recursive implementation has no provision for hitting the share as the parent, should it check for getParent() returning this?

@hierynomus
Copy link
Owner

Indeed, oversight on my part, these should've been static from the get-go.

Maybe introduce a new static method that gets called from the instance methods, and mark those deprecated. That would maintain API compatibility and allow you to call the static variants directly.

@ecki
Copy link

ecki commented Jan 8, 2023

Maybe a new static util class with proper static impl and Limited constructor Accessibility (momostate) and delegate and deprecate the old class?

@hierynomus
Copy link
Owner

Also makes sense to do! Let's go for that

hannosgit added a commit to hannosgit/smbj that referenced this pull request Mar 18, 2023
@hannosgit hannosgit mentioned this pull request Mar 18, 2023
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