-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat: Add current folder context for file list actions #1113
Conversation
Signed-off-by: Christopher Ng <chrng8@gmail.com>
Bundle ReportBundle size has no change ✅ |
Signed-off-by: Christopher Ng <chrng8@gmail.com>
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.
While I do get the point of object parameters, mixing both seems weird. I would pass the directory directly, not within an object.
We do the same in the new file menu
Next release is a minor, let's wait for this PR to be in before releasing the next minor? |
No 3.10.0 was never released so we can put it into 3.10.0 |
Damn did not saw the auto merge |
Having more than (I think 2, others say 3) parameters is often a mess, especially when this is only passing options. function doSomething(value, options) so you do not end up with a lot of parameters. But on the other hand having many parameters / options means that your function should be refactored as it is doing to many things and probably can be split... |
Aaaand now it's released... Was a bit fast and ignoring my comment @susnux |
Part of nextcloud/server#5534