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 getSourceIdsHash with a case-sensitive search will miss getSourceIDsHash, 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 getSourceIdsHash() method is defined in the Migrate module, but we should change it wherever it is used. Currently, I find one use in a test inside the Node module.

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 getSourceIdsHash core | grep -v getSourceIdsHash
grep -rni 'getSourceIdsHash.*getSourceIdsHash' 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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

benjifisher created an issue. See original summary.

maboresev’s picture

Assigned: Unassigned » maboresev

I'm working on it.

maboresev’s picture

Assigned: maboresev » Unassigned
Status: Active » Needs review
FileSize
13.81 KB

Here is the patch.

benjifisher’s picture

Status: Needs review » Reviewed & tested by the community

@maboresev: I checked your profile, and it looks as though this is your first core contribution. Congratulations! Now that you have gone through the process of dealing with the issue queue, making a patch, and so on, I hope you can start finding things about Drupal that annoy you and fixing them!

I reviewed the patch with some help from vim and verified that every change was replacing an incorrect version with the correctly capitalized version.

I tested with the grep commands in the issue summary before and after applying the patch. The first command showed many lines before the patch; the other three commands showed none.

In short, it all looks good.

maboresev’s picture

Thanks @benjifisher! I'm going to work hard to make more Drupal contributions.

alexpott’s picture

Crediting @benjifisher for creating an reviewing this issue.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed 9d0bbf5a95 to 8.6.x and a4a0818998 to 8.5.x. Thanks!

Backported to 8.5.x because PHP methods are case-insensitive and the migrate code is still getting lots of big fixes so have the code in-sync is helpful.

  • alexpott committed 9d0bbf5 on 8.6.x
    Issue #2960978 by maboresev, benjifisher: Change getSourceIDsHash to...

  • alexpott committed a4a0818 on 8.5.x
    Issue #2960978 by maboresev, benjifisher: Change getSourceIDsHash to...

Status: Fixed » Closed (fixed)

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