-
Notifications
You must be signed in to change notification settings - Fork 37
Add configurable "shared with me" output dir #285
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
Conversation
6b01d22 to
6731996
Compare
Signed-off-by: Severin Hamader <[email protected]>
6731996 to
66f52f9
Compare
lukasdotcom
left a comment
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.
Hi @Bungeefan 👋,
Thanks so much for the comprehensive pr. I tested it and it worked on my simple test accounts 🎉. I've added some feedback.
Signed-off-by: Severin Hamader <[email protected]>
Signed-off-by: Severin Hamader <[email protected]>
Signed-off-by: Severin Hamader <[email protected]>
…meter list Signed-off-by: Severin Hamader <[email protected]>
Signed-off-by: Severin Hamader <[email protected]>
Signed-off-by: Severin Hamader <[email protected]>
66f52f9 to
0548d7c
Compare
lukasdotcom
left a comment
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.
You have a failing check that can be fixed with composer cs:fix, but other then that it looks good. If there are any specific refactoring you want to do I can tell you if they make sense.
Signed-off-by: Severin Hamader <[email protected]>
|
Thanks for the review, fixed the obsolete import. As I could already remove a lot of the parameters from the If you have no further objections, feel free to merge it. |
lukasdotcom
left a comment
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.
Thanks. I'll merge it then 🚀
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Closes #277
This PR adds the requested "Shared with me" directory setting, along with the following improvements:
ownedByMecheck with a Drive API query parameterFeel free to request changes, I am myself not super happy with the implementation, but further improvements would require more refactoring, which I didn't feel comfortable starting without any feedback/guidance (therefore, I tried to change as little as possible while still preventing duplicated code).