-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Adding detailed mode to occ security:routes #28642
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
|
what about Webdav routes ? |
|
do we expect this to make it in 10.0.3 or defer to planned for 10.0.4 ? |
|
moving to "triage" |
|
done differently in #28901 |
10aafc4 to
d2408c7
Compare
butonic
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.
nice
core/Command/Security/ListRoutes.php
Outdated
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.
Is it possible to "merge" these 2 loops in order to avoid looping through the list twice?
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.
in line 74 the list of collections is compressed to on list of routes
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.
premature optimization might not be needed here, considering that this code rarely runs and the number of entries should be small
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.
I meant something like:
foreach ($this->router->getCollections() as $c) {
$new = $c->all();
foreach ($new as $name => $route) {
....
$rows[] = array_merge(.....);
}
}
need to be careful with the $c variable in this case.
core/Command/Security/ListRoutes.php
Outdated
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.
Move this implode to the row creation (lines 97 and 81) to avoid another list traversal?
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.
not really because in line 93 there is a array merge
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.
In line 100 you have sort ($rows[$path]['methods']);. You can make the implode after that unless you need the array for something else.
Adjustment will be needed for the other case (lines 81-82)
|
I'd add a comment in |
d2408c7 to
122f7be
Compare
|
@butonic pls create a documentation PR to reflect the new option |
|
please backport for 10.0.4 |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
Related Issue
refs https://github.com/owncloud/enterprise/issues/2207
Whats missing?
How Has This Been Tested?
Manual local testing ...
Screenshots (if appropriate):
Types of changes
Checklist: