file_validate_size may not need to hit DB [#197266]

Comment	File	Size	Author
#14	197266.patch	1.03 KB	drewish

#11	file_197266.patch	2.01 KB	drewish

#5	197266-2.patch	1.89 KB	c960657

#4	197266.file_validate_size.patch	1.31 KB	Dave Reid

#2	file.inc_.quota_2.patch	1.23 KB	lilou

	file.inc_.quota_.patch	1.14 KB	ufku

Comment #1

drewish CreditAttribution: drewish commented 15 April 2008 at 21:28

Version:

6.x-dev

» 7.x-dev

Log in or register to post comments

Comment #2

lilou CreditAttribution: lilou commented 24 August 2008 at 15:13

File	Size
file.inc_.quota_2.patch	1.23 KB

Reroll.

Log in or register to post comments

Comment #3

drewish CreditAttribution: drewish commented 11 September 2008 at 21:06

Status:

Needs review

» Reviewed & tested by the community

a small change to help avoid an extra query.

Log in or register to post comments

Comment #4

Dave Reid

he/him

English

Nebraska USA

CreditAttribution: Dave Reid commented 11 September 2008 at 23:05

Status:

Reviewed & tested by the community

» Needs work

File	Size
197266.file_validate_size.patch	1.31 KB

Looks like you're missing a closing brace on that if statement. Also please use cvs diff -uP so we can get the relevant function name in the diff. Here's a revised patch that also fixes the PHP doc for $user_limit.

Log in or register to post comments

Comment #5

c960657 CreditAttribution: c960657 commented 17 September 2008 at 19:29

Status:

Needs work

» Needs review

File	Size
197266-2.patch	1.89 KB

Reroll. file_validate_size() is already covered by unit tests.

Log in or register to post comments

Comment #6

17 September 2008 at 19:34

Status:

Needs review

» Needs work

Patch failed to apply. More information can be found at http://testing.drupal.org/node/14498. If you need help with creating patches please look at http://drupal.org/patch/create

Log in or register to post comments

Comment #7

drewish CreditAttribution: drewish commented 17 September 2008 at 19:39

Status:

Needs work

» Needs review

ignore DrupalTestbedBot it's buggy: #307197: Testing the incorrect patch posting multiple comments.

Log in or register to post comments

Comment #8

drewish CreditAttribution: drewish commented 18 September 2008 at 19:13

Status:

Needs review

» Reviewed & tested by the community

just re-ran the tests, they're still good. simple enough patch too.

Log in or register to post comments

Comment #9

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 19 September 2008 at 03:40

Version:

7.x-dev

» 6.x-dev

Thanks, committed! Looks like this could go in 6.x too.

Log in or register to post comments

Comment #10

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 23 September 2008 at 10:12

Status:

Reviewed & tested by the community

» Needs work

Looks like a good improvement. However (1) it does not apply to Drupal 6 (2) I'd add a comment to the if() which would explain that the function call is in there to help performance a bit. Otherwise someone might go and "prettify" if later on.

$ patch -p0 < 197266-2.patch.txt 
patching file includes/file.inc
Hunk #1 FAILED at 728.
Hunk #2 succeeded at 652 (offset -94 lines).
1 out of 2 hunks FAILED -- saving rejects to file includes/file.inc.rej