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

Added List files #754

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

Added List files #754

wants to merge 2 commits into from

Conversation

rokkakasu
Copy link

Added List files method to list all the files under the directory recursively using single session.

@hierynomus
Copy link
Owner

Hi @rokkakasu. Thank you for your effort. However, in this PR's current state, I won't merge it. Things like these do not belong in the core classes of the library. The core classes expose the base functionality. If I would merge this it would hide a huge performance penalty. These kind of things belong in classes built on top of the library. This could be added to for instance SmbFiles.

Also the code quality is well below standards of this repository. There are a lot of indentation errors.

@rokkakasu
Copy link
Author

rokkakasu commented Mar 19, 2023

Hi @hierynomus Kindly reuse the part of the code I have provided so that it will be useful for many library users.
in present implementation to list files in each subfolders a separate NTLM auth is needed,
with this methods that can be avoided.
Kindly use the Code in any form. and feel free to reject the PR.

Thanks,
@rokkakasu

Added List files method to list all the files under the directory recursively using single session.
Added Param for full file path.
as its required to identify the full path of the file in the Share
@laeubi
Copy link
Contributor

laeubi commented Apr 21, 2023

@rokkakasu I think using a Stream would have many benefits, and is used in the NIO API as well, this will allow to iterate but still not eager fetch everything.

* This utility class provides utilities for file attributes.
*
*/
public class SmbFileAttributeUtils {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you maybe submit this as a separate PR to SmbFiles this is really useful and I missing such a good way to check if its a file or directory!

@rokkakasu
Copy link
Author

rokkakasu commented Apr 21, 2023 via email

@rokkakasu
Copy link
Author

rokkakasu commented Apr 21, 2023 via email

@hierynomus
Copy link
Owner

Let me re-iterate. In the current implementation this will not be approved nor merged into the master branch. Submit a good PR, containing code, tests and proper formatting. Also not duplicating any constants already defined in the library, then I'll review and think about merging.

@hierynomus
Copy link
Owner

As said initiallly, this code does not belong in the core classes of the library, but instead in a utilities class, such as SmbFiles.

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