WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 21 months ago

#22405 closed enhancement (fixed)

Easier expression of file sizes and other data amounts

Reported by: nbachiyski Owned by: DrewAPicture
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.5
Component: General Keywords: has-patch
Focuses: Cc:

Description

In the spirit of #20987 writing 64 * 1024 * 1024 * 1024 * 1024 isn't very readable and cool.

Here is a patch, which adds a couple of constants like KB_IN_BYTES, MB_IN_BYTES, etc.

Attachments (5)

data-constants.diff (1.2 KB) - added by nbachiyski 5 years ago.
22405.patch (2.1 KB) - added by chriscct7 22 months ago.
22405.2.patch (1.9 KB) - added by sudar 22 months ago.
Refreshed patch with docblock
22405.3.patch (706 bytes) - added by sudar 21 months ago.
22405.4.patch (4.8 KB) - added by sudar 21 months ago.
Use *_IN_BYTES constants instead of hardcoded numbers

Download all attachments as: .zip

Change History (27)

#1 @SergeyBiryukov
5 years ago

  • Version changed from trunk to 3.4

#2 @SergeyBiryukov
5 years ago

  • Version changed from 3.4 to trunk

On second thought, this ticket is a continuation of #20987, so 3.5 is the earliest affected version here.

#3 @SergeyBiryukov
5 years ago

  • Keywords 3.6-early added
  • Milestone changed from Awaiting Review to Future Release

#4 @SergeyBiryukov
3 years ago

#27357 was marked as a duplicate.

#5 @chriscct7
22 months ago

  • Keywords 3.6-early removed
  • Owner set to chriscct7
  • Status changed from new to assigned

@chriscct7
22 months ago

#6 @chriscct7
22 months ago

  • Keywords commit added
  • Milestone changed from Future Release to 4.4
  • Status changed from assigned to accepted

Refreshed patch

#7 @DrewAPicture
22 months ago

  • Keywords needs-docs added; commit removed

If we're going to go the "pluggable" route on existing and new constants, we'll need to add constant DocBlocks to each definition (previously there was a multi-definition DocBlock wrapping the time constants.

#8 follow-up: @SergeyBiryukov
22 months ago

I don't think the "pluggable" route is necessary here.

#9 in reply to: ↑ 8 ; follow-up: @DrewAPicture
22 months ago

Replying to SergeyBiryukov:

I don't think the "pluggable" route is necessary here.

In that case, we should probably add a multi-definition DocBlock for the new constants. And do we really need to go above a terabyte? Seems like there wouldn't be a great deal of need for anything higher than that.

#10 in reply to: ↑ 9 @SergeyBiryukov
22 months ago

Replying to DrewAPicture:

And do we really need to go above a terabyte? Seems like there wouldn't be a great deal of need for anything higher than that.

Agreed.

#11 @DrewAPicture
22 months ago

  • Keywords needs-patch added; has-patch removed

@sudar
22 months ago

Refreshed patch with docblock

#12 @sudar
22 months ago

  • Keywords has-patch added; needs-docs needs-patch removed

I have removed constants above terabyte and have also added multi-definition DocBlock enclosing the new constants.

#13 @wonderboymusic
21 months ago

I don't like the if ( ! defined( 'PIZZA_IN_SLICES' ) ) stuff, otherwise, seems somewhat useful.

#14 @sudar
21 months ago

@wonderboymusic

Do you want to remove the if ( ! defined( 'KB_IN_BYTES' ) ) checks and just add the definition of the constants alone?

#16 @sudar
21 months ago

Refreshed patch to remove if ( ! defined( 'KB_IN_BYTES' ) ) checks.

@sudar
21 months ago

#17 @DrewAPicture
21 months ago

  • Owner changed from chriscct7 to DrewAPicture

#18 @DrewAPicture
21 months ago

In 35286:

Filesystem: Introduce constants to allow for easier expression of file sizes and other data amounts.

Adds:

  • KB_IN_BYTES (kilobytes)
  • MB_IN_BYTES (megabytes)
  • GB_IN_BYTES (gigabytes)
  • TB_IN_BYTES (terabytes)

Props nbachiyski, sudar.
See #22405.

#19 @DrewAPicture
21 months ago

  • Keywords needs-patch added; has-patch removed

OK we've got the constants. Let's use them. Needs a patch.

@sudar
21 months ago

Use *_IN_BYTES constants instead of hardcoded numbers

#20 follow-up: @sudar
21 months ago

  • Keywords has-patch added; needs-patch removed

In addition to the ones that I changed in the above patch I also found a couple of instances inside wp-includes/ID3/*.php and wp-admin/includes/class-pclzip.php.

Since these were external libraries I didn't change them to use the *_IN_BYTES constants. Let me know If the hard coded numbers should be replaced with the constants even in these external libraries.

#21 in reply to: ↑ 20 @DrewAPicture
21 months ago

Replying to sudar:

In addition to the ones that I changed in the above patch I also found a couple of instances inside wp-includes/ID3/*.php and wp-admin/includes/class-pclzip.php.

Since these were external libraries I didn't change them to use the *_IN_BYTES constants. Let me know If the hard coded numbers should be replaced with the constants even in these external libraries.

Nope, you did right to leave them alone I think. If others disagree, we can always come back and do those replacements later. Thanks for the patch.

#22 @DrewAPicture
21 months ago

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

In 35325:

Filesystem: Following the introduction of the KB|MB|GB|TB_IN_BYTES constants in [35286], use them in various places in core.

Props sudar.
Fixes #22405.

Note: See TracTickets for help on using tickets.