Closed (duplicate)
Project:
Drupal core
Version:
8.6.x-dev
Component:
batch system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
3 May 2017 at 13:04 UTC
Updated:
20 Apr 2018 at 15:44 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #4
john cook commentedI've updated the summary to make the task clearer.
Comment #5
john cook commentedComment #6
john cook commentedComment #7
rajeevkAttaching patch for config module using new batch builder.
Comment #10
james.shee commentedAttempting to work as part of Drupalcon 2018 Nashville mentored sprint
Comment #11
john cook commentedWe should be passing the class and method as the first parameter. This would be along the lines of
->setFinishCallback([ConfigSync::class, 'finishBatch'])The classes passed in do not match the currently used classes.
$this->t()should be used to make strings translatable.The class names do not match the currently used classes.
Use the
['MyClass', 'myCallbackMethod']format for the first parameter of the setFinishedCallback() and addOperation() methods.Please use
$this->t()for translatable text.Comment #12
james.shee commentedHi John, thank you for the tips. Adding new patch. This is my first commit
You stated that we should use $this->t(). Is it okay to just use t()?
https://www.drupal.org/node/2875389
Comment #13
james.shee commentedHopefully this patch works better, did a git diff this time
Comment #14
john cook commented@James.Shee, when in class methods you use
$this->t()and in functions (outside of classes)t()is used.From the StringTranslationTrait documentation:
This helps with code injection (swapping the code of equivalent interface). This is most commonly used for unit tests.
Comment #15
james.shee commentedAccidentally removed the foreach loop, and added in $this->t()
That should resolve a few of the errors, will try to figure out what is going on with the rest, just uploading for now.
Comment #16
james.shee commentedWhoops, uploaded the wrong patch
Comment #17
rajeshwari10 commentedComment #18
rajeshwari10 commentedPlease Review the patch.
Thanks!!
Comment #19
borisson_There are some whitespace problems in this patch that should be fixed.
Should be 2 spaces instead of 4?
Should stay at 2 spaces.
^
^
Comment #20
yogeshmpawarComment #21
yogeshmpawarCoding standard issues fixed as mentioned in #19& also added an interdiff.
Comment #22
rajeevkIssues pointed out in #19 is fixed in #21. It looks good now..
Comment #23
alexpottComment on meta:
Basically all of these patches should be merged there.