Make WordPress Core

Changes between Initial Version and Version 1 of Ticket #51553, comment 15


Ignore:
Timestamp:
11/17/2020 03:58:45 PM (4 years ago)
Author:
SergeyBiryukov
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #51553, comment 15

    initial v1  
    55I was discussing with @SergeyBiryukov, and here are some notes from what we discussed:
    66
    7 - There are a few name changes that we didn't love. For example, renaming `$item` to `$data_object` everywhere int eh affected code. It seems simpler to just assign `$item=$data_object;` at the beginning. This is something done in other instances already.
    8 - In some places the PR just assignes `$post = $item` at the beginning, in others it replaces everything with `$item`, which could make the code less clear. One location this is noticeable is within `class-wp-ms-themes-list-tablep.php`. The [https://github.com/WordPress/wordpress-develop/pull/612/files#diff-0dc5774897687f8570c4ae0af63be5074e83455f3382cca1db88ce9a05656554 first two methods rename the variable across the whole method, but the third one does not]. Would be great to have some consistency there.
     7- There are a few name changes that we didn't love. For example, renaming `$item` to `$data_object` everywhere int eh affected code. It seems simpler to just assign `$item = $data_object;` at the beginning. This is something done in other instances already.
     8- In some places the PR just assignes `$post = $item` at the beginning, in others it replaces everything with `$item`, which could make the code less clear. One location this is noticeable is within `class-wp-ms-themes-list-table.php`. The [https://github.com/WordPress/wordpress-develop/pull/612/files#diff-0dc5774897687f8570c4ae0af63be5074e83455f3382cca1db88ce9a05656554 first two methods rename the variable across the whole method, but the third one does not]. Would be great to have some consistency there.
    99
    1010I think task 3 is the only area here for potential issues in PHP 8 when not utilizing the named parameters feature, but to my knowledge no related problems have been reported. It's likely some will be uncovered going forward though.