-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
show a short list of mount infos: files_external:list --short #32178
Conversation
Codecov Report
@@ Coverage Diff @@
## master #32178 +/- ##
============================================
+ Coverage 64.33% 64.34% +0.01%
Complexity 18305 18305
============================================
Files 1195 1195
Lines 69193 69228 +35
Branches 1276 1276
============================================
+ Hits 44517 44548 +31
- Misses 24304 24308 +4
Partials 372 372
Continue to review full report at Codecov.
|
Maybe it's a good opportunity to add the missing tests. In general we only skip adding tests when dealing with legacy code that would require heavy refactoring to make it testable. |
I understand your point, but do you want to address all the combinations?
Second, you hit my weak point: tests (😓) I am very bad in that and have honestly more or less no clue... |
@phil-davis may I ask you for support creating tests? |
Assigned myself to help with making tests. |
50c33e4
to
0bb4c3a
Compare
@mmattel I rebased this just now, to make sure the PR is up-to-date with everything in |
@phil-davis any news on your support offer regarding tests ? |
@sharidas can you help with the unit tests part ? |
I unassigned myself - I am really not a unit test guy, and have lots of other stuff to do. I should not have offered in the first place, unit tests are more a dev team thing - sorry. |
0bb4c3a
to
0945076
Compare
Unit test status
@PVince81 I have updated the unit tests. CI looks promising. I would like to have a review for the unit tests added. PS: The testing is done with 2 mount points. |
0945076
to
bd7c913
Compare
Update unit test for short list of mount infos feature. Signed-off-by: Sujith H <sharidasan@owncloud.com>
bd7c913
to
3d7f0b1
Compare
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.
👍
@mmattel please backport |
…tput show a short list of mount infos: files_external:list --short
The example in the OP says: How Has This Been Tested? sudo -uwww-data ./occ files_external:list user_1 --short Testing in my own server: Should we show the mounts with "available for" the user? If not, Is there any other value available for Type Column apart from Personal? (cc @mmattel) |
I already thought about that one...
admin |
This is a new PR for #32098 because of a push problem killing the commit.
Description
This PR adds an option
-s --short
tosudo -uwww-data ./occ files_external:list user_id
Related Issue
Motivation and Context
The current output of
./occ files_external:list user_id
is quite big and for some purposes hard to read. This short view helps when using in conjunction offiles:scan
with option--path=
to identify easily the scannable paths for a user.How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist:
Open tasks:
@PVince81 I added the changes requested from the original PR