-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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/workflowengine #38604
Refactors "strpos" calls in /apps/workflowengine #38604
Conversation
1529669
to
4831157
Compare
c1b33fb
to
c4385ae
Compare
…ability. Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com> Signed-off-by: Faraz Samapoor <fsa@adlas.at>
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com> Signed-off-by: Faraz Samapoor <f.samapoor@gmail.com> Signed-off-by: Faraz Samapoor <fsa@adlas.at>
c4385ae
to
9d15e6e
Compare
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.
Psalm found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
Hello @come-nc, Sorry to bother you. I understand that reviewing PRs can take some time, but sometimes my PRs do not get merged after being approved and some of them have more than two approvals. Should I merge them myself once all the required checks have passed? Are there any considerations in place to merge PRs? Thank you for your assistance. |
Hey, do not hesitate to notify us if it goes unnoticed for too long. For PRs on master at least. (But according to psalm on this one ?? is used where ?: should have been since getPathInfo can return false. But then ?: I suppose would eat strings like '0'. Not sure what the clean thing is, also I did not check what getPathInfo actually returns.) |
Sure. Thanks. I applied these changes based on your suggestion in the review: I just checked the codebase. The |
Let’s leave it like that for now. |
Summary
Following #38261 and #38260, I have replaced
strpos
calls in/apps/workflowengine
namespace as well.