Make WordPress Core

Opened 7 years ago

Closed 2 years ago

Last modified 20 months ago

#40875 closed enhancement (fixed)

Add a petabyte constant

Reported by: henrywright's profile henry.wright Owned by: johnbillion's profile johnbillion
Milestone: 6.0 Priority: normal
Severity: normal Version:
Component: Formatting Keywords: has-patch has-unit-tests add-to-field-guide
Focuses: Cc:

Description

I'm hoping I'll need to use this one day on my own site :)

define( 'PB_IN_BYTES', 1024 * TB_IN_BYTES );

On a serious note, this may now (on soon) be needed by some of the largest websites.

It's used just once in core as far as I can tell, and that's in size_format().

Attachments (1)

40875.diff (1.3 KB) - added by Presskopp 7 years ago.

Download all attachments as: .zip

Change History (24)

@Presskopp
7 years ago

#1 @Presskopp
7 years ago

  • Keywords has-patch added

Well, the day will come for somebody..

This ticket was mentioned in Slack in #core by presskopp. View the logs.


6 years ago

#3 @flixos90
6 years ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to Future Release

#4 @johnbillion
2 years ago

  • Component changed from General to Formatting
  • Keywords needs-patch needs-unit-tests added; has-patch needs-testing removed
  • Milestone changed from Future Release to 6.0
  • Owner set to johnbillion
  • Status changed from new to reviewing

As size_format() works with floats and strings we might as well add PB, EB, ZB, and YB too.

#5 @henry.wright
2 years ago

1YB is a trillion trillion bytes according to Wikipedia

Yikes :)

#6 @Presskopp
2 years ago

While we're on it, what about the base 2 / base 10 thing?

#7 @johnbillion
2 years ago

The current constants and size_format() function use base 2. What's the use case for base 10 versions? I think that can go into a separate ticket if there is a need.

This ticket was mentioned in PR #2381 on WordPress/wordpress-develop by johnbillion.


2 years ago
#8

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

#9 @johnbillion
2 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 52955:

Formatting: Add support for formatting sizes as PB, EB, ZB, and YB.

Props henry.wright, Presskopp

Fixes #40875

#11 @milana_cap
2 years ago

  • Keywords needs-dev-note added

#12 @milana_cap
2 years ago

  • Keywords add-to-field-guide added; needs-dev-note removed

This ticket was mentioned in PR #3205 on WordPress/wordpress-develop by andrewserong.


20 months ago
#13

🚧 🚧 🚧 🚧 This is very much a work in progress (not yet ready for review) 🚧 🚧 🚧 🚧

This PR is the beginnings of an attempt to backport the Layout block support refactor from Gutenberg to WordPress core for the 6.1 release. The goal is to attempt to do this in parts.

  • Part 1 (this PR) is to backport the foundation for outputting centralised styles
  • Part 2 will be to then backport layout.php in a follow-up, once the prerequisite issues have been resolved

Note that in the progress of working on this PR, it also pulls in parts of unrelated changes.

### Scope

  • Include centralised Layout styles output
  • Include spacing presets where appropriate

### Still to-do

  • Migrate over tests
  • Update broken tests
  • Ensure that the disable layout styles function is migrated over, too

### Included PRs

This change backports the following changes (while also being slightly different in order to align with core files):

Note that it doesn't _entirely_ port over #40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting layout.php

Trac ticket:

Gutenberg tracking issue: https://github.com/WordPress/gutenberg/issues/43440

andrewserong commented on PR #3205:


20 months ago
#14

Update: I _think_ most of the changes are now migrated over correctly for these files, and unit tests are finally passing 🎉

One outstanding task is that I haven't yet migrated over the test for the feature level selectors that was included in https://github.com/WordPress/gutenberg/pull/42087 by @aaronrobertshaw as I wasn't too sure where or how we should best register blocks in core tests. I'll look into that further tomorrow.

CC: @tellthemachines I believe the root padding and layout changes have all been migrated over — _except_ for the changes to layout.php. The intention in this PR is to try to merge the foundations of the layout support in one step, and then in a subsequent PR merge the layout.php changes (once the style engine has been merged), so that it's a bit easier to progress.

Since tests are now passing, I'll hook up the trac ticket and switch this over to ready for view. It's quite possible that I've missed things in the process, so please do let me know if anyone catches things that should be fixed up. Thanks!

aaronrobertshaw commented on PR #3205:


20 months ago
#15

I wasn't too sure where or how we should best register blocks in core tests. I'll look into that further tomorrow.

@andrewserong, we might be able to avoid registering a new block within the bootstrap.php for the feature-level selectors test. From memory, we needed to do that because WordPress was loaded before the tests ran, meaning theme.json generated block metadata too soon effectively ignoring any block registration we did in the specific tests. Now the image block adopts the feature level selectors, it's probably time to switch the test to use that core block.

I'm out of time today but will touch base with you tomorrow on this one.

andrewserong commented on PR #3205:


20 months ago
#16

Thanks for following up, Aaron! In that case, we might be better off adding the test for the feature level selectors once the image block's block.json file is updated, which should happen as part of https://github.com/WordPress/wordpress-develop/pull/3154. I might leave the scope of this PR as-is, then, but happy to continue investigating if anyone thinks we should pursue it in this PR.

andrewserong commented on PR #3205:


20 months ago
#17

@ockham, I believe this one should be ready for final review/commit now that we've got approval from @tellthemachines (thanks!)

To recap:

  • This PR does not require the style engine changes to be merged in first, as it doesn't change layout.php, so should be able to be safely committed at any time.
  • Once the style engine PR is merged (https://github.com/WordPress/wordpress-develop/pull/3199), I can put together a separate PR that backports layout.php.
  • Once this PR and the style engine PR is merged, we can put together a separate PR to backport the spacing presets work (CC: @glendaviesnz).

hellofromtonya commented on PR #3205:


20 months ago
#18

This PR was committed in https://core.trac.wordpress.org/changeset/54159. However, the added tests from this PR failed.

During investigation, I found that this PR tests pass when the Style Engine's commit is not applied, but fails when it is.

So this PR's commit was reverted.

Next step:

  • [ ] Rebase this PR against the current trunk (which includes the Style Engines commit.
  • [ ] Then investigate to find the root cause.
  • [ ] Fix the root cause.

Then it'll be ready again for commit consideration.

ockham commented on PR #3205:


20 months ago
#19

This PR was committed in https://core.trac.wordpress.org/changeset/54159. However, the added tests from this PR failed.

FWIW, here's a link to the failing tests.

ockham commented on PR #3205:


20 months ago
#20

Sharing some more findings (also posted in Slack):

Weirdest thing, I tried rebasing locally but can’t reproduce the error here :thinking: (At least not when only running npm run test:php — --group=themes — running the full test suite now.)

Furthermore, it looks like https://github.com/WordPress/wordpress-develop/pull/3244 has the same test failures — and that PR has a totally unrelated change (introduces a new function as an alias for an existing class method).

Heisenbug? :confused:

hellofromtonya commented on PR #3205:


20 months ago
#21

Follow-up:

When applying this PR on top of the current trunk, the tests pass locally and within the CI. I opened a separate PR #3249 as a confidence check and to isolate the code changes. Tests pass ✅

During investigation, I found that this PR tests pass when the Style Engine's commit is not applied, but fails when it is.

Repeating this same process, tests pass locally in both cases ✅

This leads me to believe the issue from earlier may possibly be related to something within the wp-env remote deps, such as Docker, Composer, GitHub, etc. Whatever that something was, it appears to be resolved.

So, I'm recommitting this PR, but just to make sure, recommitting it on top of trunk.

hellofromtonya commented on PR #3205:


20 months ago
#22

Follow-up:

When applying this PR on top of the current trunk, the tests pass locally and within the CI. I opened a separate PR #3249 as a confidence check and to isolate the code changes. Tests pass ✅

During investigation, I found that this PR tests pass when the Style Engine's commit is not applied, but fails when it is.

Repeating this same process, tests pass locally in both cases ✅

This leads me to believe the issue from earlier may possibly be related to something within the wp-env remote deps, such as Docker, Composer, GitHub, etc. Whatever that something was, it appears to be resolved.

So, I'm recommitting this PR, but just to make sure, recommitting it on top of trunk.

Note: See TracTickets for help on using tickets.