Support from Acquia helps fund testing for Drupal Acquia logo

Comments

baldwinlouie’s picture

Assigned: Unassigned » baldwinlouie

taking it: baldwinlouie and sillygwailo

baldwinlouie’s picture

attaching patch

baldwinlouie’s picture

Status: Active » Needs review

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

The last submitted patch, drupal-camelcase_replace-2002368-7450032.patch, failed testing.

baldwinlouie’s picture

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

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/query/Sql.phpundefined
@@ -1084,7 +1084,7 @@ function placeholder($base = 'views') {
+  function buildCondition($where = 'where') {

Add protected access modifier in front of the function to adher to the new OOP standards.

baldwinlouie’s picture

re-rolling the patch

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

The last submitted patch, drupal-camelcase_replace-2002368-7.patch, failed testing.

baldwinlouie’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, drupal-camelcase_replace-2002368-7.patch, failed testing.

SpartyDan’s picture

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

Status: Needs review » Reviewed & tested by the community

Looks good to me now !

elvis2’s picture

Should the access modifier on this function be protected or public?

heddn’s picture

If the method is only used internally i.e. $this->{foo}, then I'd recommend protected. Otherwise, go with public as it needs to be usable by folks outside the class is apart of the public API.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 7ab5c07 and pushed to 8.x. Thanks!

elvis2’s picture

Status: Fixed » Needs review
FileSize
622 bytes

This patch only changes the function from protected to public, based on #14.

dawehner’s picture

Status: Needs review » Fixed

I don't think we need this to be public.

elvis2’s picture

@dawehner, thanks for chiming in.

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