Problem/Motivation

Trivial patch of the month but it annoys IDE users.

Proposed resolution

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#9 2273825-9.patch609 bytesrpayanm
#1 2273825.patch665 bytesdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
FileSize
2273825.patch665 bytes

moeh

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review

.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

I think we had some super weird cases with APC and php errors related to this, but I can't remember exactly if it was because we had that extends or because we didn't.

I know it was Gabor who committed this back in Prague, the commit is 383934, a follow-up of the comment as field patch: #731724: Convert comment settings into a field to make them work with CMI and non-node entities

And that is the exact opposite of this patch, so while I agree that it should be like this, we should be very sure that it is not going to break anything anymore. See https://drupal.org/node/731724?page=1#comment-7910805, which points to https://bugs.php.net/bug.php?id=49472. That bug report is very old, so 5.4 is probably not affected but maybe we should run this on a bunch of bots to verify, or something?

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

1: 2273825.patch queued for re-testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

no longer applies.

Status: Needs work » Needs review

mgifford queued 1: 2273825.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 1: 2273825.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +Novice

This seems to be a good novice task to be honest.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented
Status: Needs work » Needs review
FileSize
2273825-9.patch609 bytes

Trying...

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
CreditAttribution: claudiu.cristea commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

Go!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed e3964f9 on 8.0.x 
    Issue #2273825 by rpayanm, dawehner: Fixed UserStorageInterface should...

Status: Fixed » Closed (fixed)

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