Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kenianbei’s picture

Assigned: Unassigned » kenianbei
misselbeck’s picture

Assigned: kenianbei » misselbeck

Working on DrupalCon 2013 Sprint

kenianbei’s picture

Assigned: misselbeck » kenianbei
Status: Active » Needs review
FileSize
1.08 KB
misselbeck’s picture

Assigned: kenianbei » Unassigned
FileSize
1.15 KB
misselbeck’s picture

Ooops...double patch.

kenianbei’s picture

Assigned: Unassigned » kenianbei

No problem, just make sure that the issue hasn't been taken by someone before you take it on. You also should check out this page on patch naming conventions: http://drupal.org/node/707484

git diff 7.x-1.x > [project_name]-[short-description]-[issue-number]-[comment-number].patch

Status: Needs review » Needs work

The last submitted patch, 2003266_changetoCamelCase.patch, failed testing.

heddn’s picture

Failing tests. Please test and re-roll.

iStryker’s picture

Status: Needs work » Needs review
iStryker’s picture

Shouldn't the function be rename as either a public or a protected function?

heddn’s picture

If it is only used internally i.e. $this->{foo} then protected seems appropriate. Otherwise use public.

oenie’s picture

#4: 2003266_changetoCamelCase.patch queued for re-testing.

iStryker’s picture

Assigned: kenianbei » Unassigned
FileSize
640 bytes
1.09 KB

Following @heddn advice, making the function public.

dawehner’s picture

Status: Needs review » Needs work

Following the advice of heddn I would argue to make it protected.

I also can't see a reason why this method would be called outside of filters.

SpartyDan’s picture

Status: Needs work » Needs review
FileSize
1.09 KB

function changed to protected

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 51777f2 and pushed to 8.x. Thanks!

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