-
Notifications
You must be signed in to change notification settings - Fork 180
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
base: master
Are you sure you want to change the base?
Added List files #754
Conversation
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 Also the code quality is well below standards of this repository. There are a lot of indentation errors. |
Hi @hierynomus Kindly reuse the part of the code I have provided so that it will be useful for many library users. Thanks, |
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
@rokkakasu I think using a |
* This utility class provides utilities for file attributes. | ||
* | ||
*/ | ||
public class SmbFileAttributeUtils { |
There was a problem hiding this comment.
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!
Sure.
…On Fri, 21 Apr, 2023, 12:58 pm Christoph Läubrich, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/main/java/com/hierynomus/smbj/utils/SmbFileAttributeUtils.java
<#754 (comment)>:
> + *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.hierynomus.smbj.utils;
+
+/**
+ * This utility class provides utilities for file attributes.
+ *
+ */
+public class SmbFileAttributeUtils {
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!
—
Reply to this email directly, view it on GitHub
<#754 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIJCEGXJ476X3Y6KUPMBXY3XCIZJFANCNFSM6AAAAAAWAC7M2Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
Hi @hierynomus/smbj ***@***.***>
May i move this method from diskshare to Smbfiles class .
Will you approve my PR.
This utility can through all sub folders without creating new sessions for
each sub directories.
Thanks
@rokkakasu.
…On Sun, 19 Mar, 2023, 8:13 pm Jeroen van Erp, ***@***.***> wrote:
Hi @rokkakasu <https://github.com/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.
—
Reply to this email directly, view it on GitHub
<#754 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AIJCEGTHIRHHNGTQDNUPFO3W44LQRANCNFSM6AAAAAAWAC7M2Q>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
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. |
As said initiallly, this code does not belong in the core classes of the library, but instead in a utilities class, such as |
Added List files method to list all the files under the directory recursively using single session.