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
-roption means to search recursively (insidecore/). - The
-noption asksgrepto show the line numbers. - The
-ioption 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
-voption 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
| Comment | File | Size | Author |
|---|---|---|---|
| #3 | migration-change_getsourceIDsHash-2960978-3.patch | 13.81 KB | maboresev |
Comments
Comment #2
maboresev commentedI'm working on it.
Comment #3
maboresev commentedHere is the patch.
Comment #4
benjifisher@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
grepcommands 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.
Comment #5
maboresev commentedThanks @benjifisher! I'm going to work hard to make more Drupal contributions.
Comment #6
alexpottCrediting @benjifisher for creating an reviewing this issue.
Comment #7
alexpottCommitted 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.