Changes between Version 2 and Version 3 of Ticket #51742, comment 13
- Timestamp:
- 11/11/2020 08:56:02 AM (4 years ago)
Legend:
- Unmodified
- Added
- Removed
- Modified
-
Ticket #51742, comment 13
v2 v3 1 1 Replying to [comment:9 audrasjb]: 2 2 3 Suggestions :3 Suggestions and nitpicks: 4 4 5 5 1. The `$major_optin_disabled = false;` var is a "double negative". Perhaps would be easier to read/understand if it was a "positive", i.e. `$major_optin_enabled = true;`? Also the name could be a bit more descriptive, perhaps, like `$upgrade_major_optin_enabled` or similar. … … 7 7 2. The `$is_major_optin_disabled` doesn't seem needed? If there should be a specific filter for disabling major core updates that runs in addition to `$upgrade_major = apply_filters( 'allow_major_auto_core_updates', $upgrade_major );`, it can perhaps be checked right in the conditional (and expect only boolean `true`). 8 8 9 3. A nitpick, perhaps `do_action( 'after_core_auto_updates_settings_fields', $auto_update_settings );` can be only `after_core_auto_updates_settings` as there aren't any "fields" there.9 3. Perhaps `do_action( 'after_core_auto_updates_settings_fields', $auto_update_settings );` can be only `after_core_auto_updates_settings` as there aren't any "fields" there. 10 10 11 11 4. The `auto_update_core_major` and similar site options would perhaps be better as explicitly set to `enabled` or `disabled` strings. It's more readable and, (as PHP 8.0 explicitly requires) it would be better to save and then expect a string, not a boolean that gets saved in the DB as a number, etc. Also, these new options should be pre-populated when updating to WP 5.6.