Problem/Motivation

We should follow coding standards and use proper camelCase for our method names: "Ids", not "IDs".

PHP is case-insensitive, so the code will work with or without this change. Anyone searching for getConnectionId with a case-sensitive search will miss getConnectionID, so fixing this will save some headaches. It is also possible that, at some point, we will do a bulk search-and-replace involving these method names.

The getConnectionId() method is defined in ConnectionUnitTest.php, but we should change it wherever it is used. Currently, I do not see any use outside of that test.

Proposed resolution

Search for all instances of the incorrect capitalization and replace them with the correct capitalization. Something like this should be effective:

grep -rni getConnectionId core | grep -v getConnectionId
grep -rni 'getConnectionId.*getConnectionId' core

For those less familiar with grep:

  • The -r option means to search recursively (inside core/).
  • The -n option asks grep to show the line numbers.
  • The -i option means to ignore case, so search for all variants.
  • The | (pipe) operator sends the output of the first command to the input of the second.
  • The -v option inverts the results, filtering out any lines that match.
  • The second and fourth commands are there just in case there are lines that have the string more than once, and one of them has the correct capitalization. Any such lines would be missed by the first and third command.

Remaining tasks

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#2 cs-method-2960981-2.patch2.13 KBjtriguero

Comments

benjifisher created an issue. See original summary.

jtriguero’s picture

Status: Active » Needs review
StatusFileSize
new2.13 KB

Hi, here is the patch.

benjifisher’s picture

Status: Needs review » Reviewed & tested by the community

@jtriguero: It looks as though this is your first contribution to Drupal core. Congratulations!

I ran the grep commands in the issue summary before and after applying the patch, with the expected results.

I also viewed the patch with my text editor and checked that the only changes are replacing an incorrectly capitalized method name with a correctly capitalized one.

In short, it all looks good. Thanks!

jtriguero’s picture

Thanks @benjifisher, I hope it will not be the last.

alexpott’s picture

Crediting @benjifisher for creating the issue and reviewing the patch.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 927e9f0 and pushed to 8.6.x. Thanks!
Committed 426f811 and pushed to 8.5.x. Thanks!

Backported to 8.5.x since this is test only.

  • alexpott committed 927e9f0 on 8.6.x
    Issue #2960981 by jtriguero, benjifisher: Change getConnectionID to...

  • alexpott committed 426f811 on 8.5.x
    Issue #2960981 by jtriguero, benjifisher: Change getConnectionID to...

Status: Fixed » Closed (fixed)

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