WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 4 years ago

Last modified 4 years ago

#13418 closed defect (bug) (maybelater)

Smaller Bits of Code Improvement

Reported by: hakre Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: General Keywords: has-patch
Focuses: Cc:

Description

This ticket is for smaller bits of code-improvements all over trunk whenever something pops into sight.

Descriptions are put in the Description field of the attachment. Feel free to add your own so this won't clutter up tac too much.

Discussion in IRC and/or wp-hackers.

Attachments (6)

13418.patch (2.0 KB) - added by hakre 8 years ago.
Code streamlined in beginning of media_upload_library_form()
13418.script-loader-max-upload-size.patch (842 bytes) - added by hakre 8 years ago.
13418.admin-tempate-wp_convert_hr_to_bytes.patch (1.1 KB) - added by hakre 8 years ago.
Following the technical specification of shorthand parsing on php.net
13418.2.diff (2.0 KB) - added by MikeHansenMe 5 years ago.
refresh based on original minus shorthand.
13418.script-loader-max-upload-size.2.patch (681 bytes) - added by MikeHansenMe 5 years ago.
refreshed based on previous patch
13418.wp_convert_hr_to_bytes.2.patch (928 bytes) - added by MikeHansenMe 5 years ago.
refreshed patch. Code now found in different file.

Download all attachments as: .zip

Change History (20)

@hakre
8 years ago

Code streamlined in beginning of media_upload_library_form()

#1 @sivel
8 years ago

So far the patch on this ticket is an "Enhancement" and is not a bug. In addition at this point of the 3.0 development, enhancements should be punted to the next release.

We could sit here for months making little enhancements when we should be focusing on bugs and blockers. The goal is to get them remaining bugs and blockers taken care of so we can get 3.0 released.

For the time being I am leaving this ticket as is, and will allow for a second opinion before punting.

#2 @sivel
8 years ago

Just wanted to take a second to apologize, I just realized you do indeed have it in the right milestone. I transposed those 2 fields when looking at it. Continue on ;)

#3 @Denis-de-Bernardy
8 years ago

I don't mean to stir a knife in a healing wound, but I'm still in awe at how much of a confrontation went on in #13317; just to get a couple of minor bits of code into WP... In many ways, I got the impression of watching a house wife telling her husband to get lost because the latter was try to show her how to do a household chore in a slightly more productive manner.

That "we've more important things to do at the moment" is, I think, unsatisfying. When in alpha, the argument is that we're adding features and fixing critical bugs that crept into the last release; when in beta/RC, the argument becomes that we're fixing important bugs in order to meet an artificial deadline. In the end, it's never time to take care of small bugs and improvements; and yet, there are so many to look into... OK, I'll stop; I'm beating a dead horse.

Still, 13418.patch doesn't change a thing as far as the logic is concerned. It merely rewrites a messy looking code block into something that looks tidy. I've no idea what should keep it from getting checked in.

Both of these tickets, in my opinion, break down to two yes|no questions: does it change any logic? If not, commit. If so, does the change that crept in make any sense (e.g. does it make sense to drop negative page numbers or font sizes)? If so, commit; if not, ask. End of story.

#4 @Denis-de-Bernardy
8 years ago

Adding to the previous note, and as much as I hate to be pointing this out, the lengthier discussions in tickets are frequently the result of committers, rather than trac trolls. Offering comments such as this one:

http://core.trac.wordpress.org/ticket/13317#comment:2

... seems much less constructive than hakre's bid to improve the quality of the WP code base. The correct way to handle these tickets is to commit the improved patch and begone with it.

#5 @hakre
8 years ago

A minor improvement for the file upload max filesize check. Might not work as intended and I could find no use of that variable. Dead code? That for the log.

@hakre
8 years ago

Following the technical specification of shorthand parsing on php.net

#6 @hakre
8 years ago

that implementation looked better to me, adopted from: Example #1

#7 @hakre
8 years ago

While doing this, stumbled over that: #13575

#8 @hakre
8 years ago

#10814 -> 10814-inter-3-0.patch
#5998 -> 5998-interrim.patch

#9 @hakre
8 years ago

No that small but an improvement as well: #11598

#10 @dd32
7 years ago

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

Patches look generally good, Need to check if they're stale/etc.

#11 @MikeHansenMe
5 years ago

  • Cc mdhansen@… added
  • Keywords needs-refresh added; 3.2-early removed

All the patches are stale. None of them could be applied to 3.5-RC4.

@MikeHansenMe
5 years ago

refresh based on original minus shorthand.

@MikeHansenMe
5 years ago

refreshed based on previous patch

@MikeHansenMe
5 years ago

refreshed patch. Code now found in different file.

#12 @MikeHansenMe
5 years ago

  • Keywords needs-testing added; needs-refresh removed

#13 @c3mdigital
4 years ago

  • Keywords needs-testing removed
  • Resolution set to maybelater
  • Status changed from new to closed

Hopefully everyones wounds have healed :) This is a refactoring ticket with no clear focus. If something needs to be refactored is needs a clear reason, along with performance and unit tests. See: http://make.wordpress.org/core/2011/03/23/code-refactoring/

#14 @SergeyBiryukov
4 years ago

  • Milestone Future Release deleted
Note: See TracTickets for help on using tickets.