Closed (fixed)
Project:
Commerce Simple Stock
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
1 Apr 2019 at 11:18 UTC
Updated:
25 Apr 2019 at 09:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
govind.maloo commentedComment #3
pcambraLooks generally good, just one thing:
I think the labels are mismatched here
Comment #4
kkalaskar commented@govindmaloo
Changes implemented suggested by @pcambra.
Also one more thing, above drupal 8.5 community added MessengerTrait::$messenger in FormBase. It provides a trait for the messenger service. No more need to inject service manually.
FormBase -> https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Form%21Fo...
MessengerTrait -> https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Messenger...
Comment #5
kkalaskar commentedComment #6
pcambraThanks @kkalaskar, feel free to implement the trait if you want, we can just add a dependency > 8.5 in the info file.
I don't think the t() call here is needed, it would translate twice
Comment #7
maliknaik commentedFormBaseclass has messenger trait added.Comment #8
maliknaik commentedPatch with t() call removed as mentioned in comment #6
Comment #9
maliknaik commentedComment #10
kellyimagined commentedPatch from #8 works well
Comment #11
kellyimagined commentedComment #12
kkalaskar commentedHello everyone,
Added minimum version in info, composer.json file of drupal core.
Comment #13
kkalaskar commentedComment #14
pcambraThe current patch doesn't apply to the latest dev.
Comment #15
facine commentedHi @pcambra, attach a new patch which applies to the latest dev.
Comment #17
pcambraThanks all!