Skip to content
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

Refactors "strpos" calls in /apps/dav #38610

Merged

Conversation

fsamapoor
Copy link
Member

Summary

Following #38261 and #38260, I have replaced strpos calls in /apps/dav namespace as well to improve code readability.

@szaimen szaimen added 3. to review Waiting for reviews technical debt labels Jun 2, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Jun 2, 2023
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Micro optimization: use use function str_starts_with and use function str_contains so PHP doesn't have to check if there are function with that name in the current namespace before it checks globally

@fsamapoor
Copy link
Member Author

Nice!

Micro optimization: use use function str_starts_with and use function str_contains so PHP doesn't have to check if there are function with that name in the current namespace before it checks globally

Thanks for the clarification. I wasn't sure if I should keep those since there were only a couple of files that were taking advantage of this micro-optimization technique. Should I update all changed files and add use statements?

@ChristophWurst
Copy link
Member

It is nice to have but not a requirement

fsamapoor and others added 2 commits June 12, 2023 09:46
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
Co-authored-by: Thomas Citharel <tcit@tcit.fr>
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
@szaimen szaimen force-pushed the replace_strpos_calls_in_dav_app branch from a3a6dfb to 8c64cca Compare June 12, 2023 07:46
@szaimen szaimen added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jun 12, 2023
Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com>
@fsamapoor fsamapoor requested a review from ChristophWurst July 10, 2023 07:14
@fsamapoor
Copy link
Member Author

Does anything need to be done for this to get merged? It's been more than three months.

@ChristophWurst ChristophWurst merged commit 9627176 into nextcloud:master Sep 17, 2023
@fsamapoor fsamapoor deleted the replace_strpos_calls_in_dav_app branch March 8, 2024 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants