Make coder args editable by review methods [#1106678]

A couple of commits on March 7th introduced the #cache property in $coder_args. This allowed review methods to prevent Coder from caching review results. The problem is that $coder_args itself was never editable by review methods, because it wasn't passed on as a reference to do_coder_reviews(). The attached patches fixes this problem for versions 6.x-2.x and 7.x-1.x.

Comment	File	Size	Author
	editable_coder_args_71_00.patch	719 bytes	Xano
	editable_coder_args_62_00.patch	632 bytes	Xano

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

stella CreditAttribution: stella commented 15 April 2011 at 18:34

I'm not sure that patch would solve that particular problem though as the individual methods (e.g. do_coder_review_regex()) can't modify that array and it isn't modified anywhere in the function. I'd also be a bit concerned about one review method modifying the settings for the following reviews. What about just implementing hook_coder_review_args_alter()?

Comment #2

Xano

he/they

English

Southampton

CreditAttribution: Xano commented 15 April 2011 at 19:41

Status:

Needs review

» Needs work

It definitely works, but I hadn't noticed that one review method gets the same arguments as are passed on to the other methods. You explicitly added the #cache property for this in Chicago. I must have misunderstood its use then.

While writing those patches I thought it would be best to allow modules to set #cache in hook_reviews() directly. What do you think?

Comment #3

Xano

he/they

English

Southampton

CreditAttribution: Xano commented 5 May 2011 at 20:00

Bump.

While writing those patches I thought it would be best to allow modules to set #cache in hook_reviews() directly. What do you think?

Comment #4

stella CreditAttribution: stella commented 31 May 2011 at 19:50

No, not in hook_reviews(). That's not how I envisaged it and it would require a larger architecture change to allow them to do that. The caching get and set happens at a much higher level and can't be currently set on a per rule basis. I'm not sure that's even such a good idea as we'd have to cache on a per-module per-file per-review per-rule basis which would require a lot of cache get/set checks and would also kinda negate the benefit of caching.

I still think hook_coder_review_args_alter() is the best place.