Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
SourcePluginBase::count() says:
* Return a count of available source records, from the cache if appropriate.
* Returns -1 if the source is not countable.
We should define a constant on MigrateSourceInterface to hold this values for better readability.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
Comment | File | Size | Author |
---|---|---|---|
#12 | interdiff_9-12.txt | 1.16 KB | swatichouhan012 |
#12 | 3215836-12.patch | 4.56 KB | swatichouhan012 |
#9 | 3215836-9.patch | 3.26 KB | guilhermevp |
#6 | interdiff-4-6.txt | 2.91 KB | imalabya |
#6 | 3215836-6.patch | 3.25 KB | imalabya |
Issue fork drupal-3215836
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
guilhermevp CreditAttribution: guilhermevp at CI&T commentedSending patch please review.
Comment #4
guilhermevp CreditAttribution: guilhermevp at CI&T commentedSorry, didn't notice the automatically added
use
. Please review.Comment #5
joachim CreditAttribution: joachim as a volunteer commentedThanks for the patch! Looks good, just a couple of tweaks:
I would say 'source' rather than 'instance' here.
This comment should stay, but use the constant with the fully-qualified interface name instead of -1.
Comment #6
imalabyaAdded a patch to address review comments on #5
Comment #7
guilhermevp CreditAttribution: guilhermevp at CI&T commentedAwesome, thanks @imalabya! Moving to RTBC!
Comment #8
quietone CreditAttribution: quietone as a volunteer commentedThanks for working on this. Remember to check the Coding standards.
Needs to be wrapped at 80 columns.
Needs to be wrapped at 80 columns.
Comment #9
guilhermevp CreditAttribution: guilhermevp at CI&T commentedThanks quietone! Here is a patch addressing #8.
Please, review.
Comment #10
larisse CreditAttribution: larisse at CI&T commentedThe patch #9 was applied and works ok. The Coding standards was fixed too.
Comment #11
alexpottThere are a couple of instances of
core/modules/migrate/tests/src/Unit/MigrateSourceTest.php - I think these should be updated to use the new constant too.
Comment #12
swatichouhan012 CreditAttribution: swatichouhan012 at Valuebound for Valuebound commentedCreated new patch to Update with new constant in core/modules/migrate/tests/src/Unit/MigrateSourceTest.php.
please review.
Comment #13
joachim CreditAttribution: joachim as a volunteer commentedLGTM.
I applied the patch and did a search for '-1' in Migrate module and in all files with filepath matching 'Plugins/migrate' and found nothing that looks like a source count.
Comment #14
alexpottCommitted 7de8fc1 and pushed to 9.3.x. Thanks!