We're working towards replacing assertEqual with assertEquals. assertEquals requires the arguments to be ($expected, $actual, ...), whereas assertEqual requires ($actual, $expected,...).
In preparation for the final cleanup, change all assertEqual calls to have ($expected, $actual, ...) like assertEquals so that in a follow up we can just search/replace the method name.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | interdiff_18_20.txt | 1.58 KB | anmolgoyal74 |
| #20 | 3193955-20.patch | 1.11 MB | anmolgoyal74 |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !264
Comments
Comment #2
mondrakeComment #3
daffie commentedIs it an idea to split this issue up into multiple parts to make reviewing a bit easier. Say 200kb pieces?
Comment #5
mondrakeI am on this. #3 is not easy, unfortunately. An initial change can be scripted, but then like as in the assertIdentical conversion issue, many calls to assertEqual in HEAD are already with the args order that is expected for assertEquals, so we need to git add them after manual review. Let's see where we land.
Comment #6
mondrakeFor review. It's a large change, still exclusively swapping the first and the second arguments of
assertEqualcalls, where appropriate. No other changes.Reviewers, please only address the following question: is the first argument now corresponding to what we expect the correct value should be? If not, we have to revert the change.
It's just too easy to fall in the pitfall of pursuing more optimizations here (there's plenty...) but we should stick to this minimum scope.
Comment #7
mondrakeFixes according to comments in MR
Comment #8
daffie commentedOne nitpick left.
Comment #9
mondrakeFixed
Comment #10
daffie commentedAll the changes look good to me.
For me it is RTBC.
Comment #11
catchReviewed with --color-words. I found one issue in breadcrumb module (see comment on the MR). But I only reviewed up until the end of image module so far.
Comment #12
mondrakeReverted BookBreadcrumbTest changes.
Comment #13
catchGot to the end of the MR and found another couple I think.
Comment #14
mondrakeThank you for your reviews @daffie @catch... I know it's a big one to work on.
Comment #16
catchCommitted afd86e7 and pushed to 9.2.x. Thanks!
Needs a re-roll/rebase for 9.1.x
Comment #18
mondrakeReroll for 9.1
Comment #20
anmolgoyal74 commentedThe view display name needs to be changed to "Master" in
core/modules/views/tests/src/Functional/Plugin/DisplayTest.php9.1.x. This has been modified in #3186582: Replace the word "master" with "default" in Views in 9.2.x only.Here's the link to the commit
https://git.drupalcode.org/project/drupal/-/commit/21bd33d6ce9adebedd61b...
Comment #21
anmolgoyal74 commentedComment #22
daffie commentedI used the linux command "diff" to see what the differences are between the 2 patch files. All the differences are expected, therefore for me it is RTBC.
Comment #23
catchCommitted f56797a and pushed to 9.1.x. Thanks!
Comment #25
catch