Skip to content

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

@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
@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