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
Fixing unsafe comparison warnings (==
=> ===
, !=
=> !==
)
#18195
Conversation
==
=> ===
, !=
=> !==
)
… comparison * changed if condition to check wether strpos response is different to false
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.
Left a couple of comments. In general it might not be that easy to change ==
to ===
in all cases. Unfortunately our travis tests are currently not running as expected. As soon as they will work again we can trigger a new build to see how many tests are failing...
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
…mo into unsafe-comparison-problem-fix
…erly. Added casting to int/string to pass strict comparisons
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.
Had another look through the changes.
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
…mo into unsafe-comparison-problem-fix
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.
Found a couple more things. But besides that it's looking fine. We may need to wait until mid of next week till the travis tests are running again. Once we had them running we can check if anything causes problems....
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
Co-authored-by: Stefan Giehl <stefan@matomo.org>
@hywak would you mind merging in the latest changes from 4.x-dev branch. Travis builds should be up and running again, so we hopefully can see afterwards if all those changes break anything or not. |
…mo into unsafe-comparison-problem-fix
@hywak Seems there are a plenty of tests failing. Might be good to first looking into why the Unit tests are failing, maybe fixing those might also fix some of the other test suites: |
Co-authored-by: Stefan Giehl <stefan@matomo.org>
@sgiehl I will take a look at it tomorrow or at the weekend :) |
@sgiehl Is there documentation on how to run tests locally? |
@hywak sure. See https://developer.matomo.org/guides/tests |
Pushed changes where all unit tests were passing locally: 45a3aa5 |
The System tests are still failing though. Seems to be caused by some kind of rounding issue. Guess some check doesn't do the same as before... |
If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'. |
This PR was last updated more than one month ago, maybe it's time to close it. Please check if there is anything we still can do or close this PR. ping @matomo-org/core-reviewers |
@hywak I will close this one now. I don't think we will be able to easily get this PR into a state where we can safely merge. The number of changes is simply to high to identify which are causing the test failures. |
Description:
#17686
Lot's of files are changed, but those are cosmetics changes only.
Review