Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lokapujya’s picture

Assigned: Unassigned » lokapujya
lokapujya’s picture

Drupal Portland Code Sprint!

lokapujya’s picture

Status: Active » Needs review
munizjor’s picture

Status: Needs review » Needs work

Patch is 0 bytes.

mcpuddin’s picture

Reroll patch -- 0kb for some reason

lokapujya’s picture

Thanks for reviewing. Patch fixed.

lokapujya’s picture

Status: Needs work » Needs review
mcpuddin’s picture

Status: Needs review » Reviewed & tested by the community

applied patch, tested views, worked well! After robot thing goes through ready to go in!

Status: Reviewed & tested by the community » Needs work
Issue tags: -Novice, -VDC

The last submitted patch, 2002972-rename-views-method.patch, failed testing.

lokapujya’s picture

Status: Needs work » Needs review
Issue tags: +Novice, +VDC

#6: 2002972-rename-views-method.patch queued for re-testing.

elvis2’s picture

Status: Needs review » Needs work

You need to add the access modifier (public/private/protected) before the function name. Take a look at the parent issue (http://drupal.org/node/1856630), comments #28, #34, #35 for more details.

You can see an example patch here: http://drupal.org/node/2001672

tvlooy’s picture

Status: Needs work » Needs review
FileSize
6.55 KB

Added the access modifiers.

elvis2’s picture

@tvlooy thanks for working on this.

I went ahead and re-rolled the patch since there were other instances of tokenize_value() being used. I believe you only checked the views module? When we rename the functions we need to grep across the whole drupal_root directory. Sometimes other modules run tests that reference functions that do not belong to the module calling the function.

Status: Needs review » Needs work
Issue tags: -Novice, -VDC

The last submitted patch, 2002972-13-rename-views-method.patch, failed testing.

elvis2’s picture

Status: Needs work » Needs review
Issue tags: +Novice, +VDC
lokapujya’s picture

The patch in #13 changes 2 instances of render_trim_text to RenderTrimText which is actually handled by a separate issue. The patch in #12 is the correct patch.

elvis2’s picture

@lokapujya I apologize about that, I must have not reset the git repo before making the patch.

The reason I rolled #13 was because #12 was missing a few renames of tokenize_value(). So #12 and #13 need to be rerolled. If this is not rerolled by this evening I will work on it.

Thanks for catching the problem.

oenie’s picture

Status: Needs review » Needs work

Unfortunately we need a reroll to the newest Drupal core as well, the patch doesn't apply anymore.

lokapujya’s picture

Status: Needs work » Needs review
FileSize
6.56 KB

Re-rolled. I found 11 instances of tokenize_value(). They were all within the Views module.

elvis2’s picture

Status: Needs review » Reviewed & tested by the community

@lokapujya thanks for re-rolling. Looks good.

Reviewed patch based on the following criteria: http://drupal.org/node/1856630#comment-7450696

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Needs a reroll

curl http://drupal.org/files/2002972-19-rename-views-method.patch | git a
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  6717  100  6717    0     0  12790      0 --:--:-- --:--:-- --:--:-- 19246
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/area/Text.php:67
error: core/modules/views/lib/Drupal/views/Plugin/views/area/Text.php: patch does not apply
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/area/TextCustom.php:61
error: core/modules/views/lib/Drupal/views/Plugin/views/area/TextCustom.php: patch does not apply
error: patch failed: core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php:340
error: core/modules/views/lib/Drupal/views/Plugin/views/field/FieldPluginBase.php: patch does not apply
jibran’s picture

Status: Needs work » Needs review
FileSize
6.6 KB

Reroll

lokapujya’s picture

Status: Needs review » Reviewed & tested by the community

GTG.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 8612773 and pushed to 8.x. Thanks!

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