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 (insidecore/
). - The
-n
option asksgrep
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
Comment | File | Size | Author |
---|---|---|---|
#3 | migration-change_getsourceIDsHash-2960978-3.patch | 13.81 KB | maboresev |
Comments
Comment #2
maboresev CreditAttribution: maboresev at SDOS commentedI'm working on it.
Comment #3
maboresev CreditAttribution: maboresev at SDOS 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
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.
Comment #5
maboresev CreditAttribution: maboresev at SDOS 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.