This patch converts the node module please look over it closely for mistakes or misinterpretations i might have made concerning the point of this antipattern resolution and ill do the rest this week.

This patch:
* replaces a number pf plain statics with the new function based approach for consistency
* Removes the $reset argument from the node_get_types function and moves the reset_cache operation under types_rebuild() and _node_revision_access() as appropriate

CommentFileSizeAuthor
#19 423822-node-static.patch1.5 KBagentrickard
#17 423822-node-static.patch1.48 KBagentrickard
#14 423822-14.patch1.51 KBJamesAn
#12 423822-12.patch9.94 KBJamesAn
#5 423822-5.patch9.94 KBJamesAn
#10 423822-10.patch9.94 KBJamesAn
#3 423822-3.patch1.86 KBJamesAn
#1 static_cache_node.patch3.82 KBmichaelfavia
static_cache_node.patch3.71 KBmichaelfavia
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

michaelfavia’s picture

michaelfavia CreditAttribution: michaelfavia commented
FileSize
static_cache_node.patch3.82 KB

forgot to name "extras" in reset. makes a good argument for storing them as assoc array and unsetting the parent.

Status: Needs review » Needs work

The last submitted patch failed testing.

JamesAn’s picture

JamesAn CreditAttribution: JamesAn commented
Assigned: michaelfavia » Unassigned
Status: Needs work » Needs review
FileSize
423822-3.patch1.86 KB

Here's an updated attempt. The node module has been modified since the last attempt.

Status: Needs review » Needs work

The last submitted patch failed testing.

JamesAn’s picture

JamesAn CreditAttribution: JamesAn commented
Status: Needs work » Needs review
FileSize
423822-5.patch9.94 KB

I thought this patch would be a bit harder.. =P

The last submitted patch failed testing.

JamesAn’s picture

JamesAn CreditAttribution: JamesAn commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch failed testing.

lilou’s picture

lilou CreditAttribution: lilou commented
Status: Needs work » Needs review

Setting to 'needs review' - testbot was broken.

JamesAn’s picture

JamesAn CreditAttribution: JamesAn commented
FileSize
423822-10.patch9.94 KB

Rerolled. Several of the changes were offset by a fair bit.

Status: Needs review » Needs work

The last submitted patch failed testing.

JamesAn’s picture

JamesAn CreditAttribution: JamesAn commented
FileSize
423822-12.patch9.94 KB

Rerolled.

Status: Needs review » Needs work

The last submitted patch failed testing.

JamesAn’s picture

JamesAn CreditAttribution: JamesAn commented
Status: Needs work » Needs review
FileSize
423822-14.patch1.51 KB

Rerolled. A lot of the changes are no longer necessary as node_load uses entity_load that doesn't use static vars.

Status: Needs review » Needs work

The last submitted patch failed testing.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented

Re-rolled.

Any reason for the array() values being passed in the original patch?

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
FileSize
423822-node-static.patch1.48 KB

And the patch.

agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
Status: Needs work » Needs review
agentrickard’s picture

agentrickard
he/him
Primary language English
Location Georgia (US)
CreditAttribution: agentrickard commented
FileSize
423822-node-static.patch1.5 KB

I see why we need the array() params. Put back.

HedgeMage’s picture

HedgeMage CreditAttribution: HedgeMage commented

I just tested the patch from #17 and it looks good to me.

cburschka’s picture

cburschka
they
CreditAttribution: cburschka commented
Status: Needs review » Reviewed & tested by the community

Not quite; the array() is indeed necessary to avoid notices. #19 seems ready, however.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Wow, this is a straggler. Committed to HEAD, thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.