Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
forms system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
27 Aug 2014 at 19:55 UTC
Updated:
16 Sep 2014 at 13:30 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
tim.plunkettComment #2
dawehnerDid you considered to mark them as private?
Let's put some documentation on there.
Can't you instead provide an optional constructor dependency? How does that work, to not fail? This code assumes that the current user always exists.
Comment #3
tim.plunkettI had this as private locally, reverted it at the last minute :)
I copied currentUser() from FormBuilder, but you're right, we don't need to do that.
Comment #4
jibranIt is simple re-factoring so +1 but I must say new test coverage is amazing. Just minor issues need some clarification other then that RTBC.
Please move this above the line like all other inline comments.
Why not use simple getMock()?
Comment #5
tim.plunkett1) This is the same comment format as the only other public: false, and
2) Because that is only valid for interfaces,... which we didn't have when the FormBuilder tests went in but we do have now (as of #2160495: Add a KeyValueExpirableFactoryInterface).
Comment #6
jibranThank you for explaining that.
Comment #8
webchickCommitted and pushed to 8.x. Thanks!
Comment #9
tim.plunkettThanks!
Comment #10
berdirGetting those errors now when running unit tests:
Comment #11
sun@Berdir: See #2329765: Remove all @backup* annotations from (pure) unit tests