-
Notifications
You must be signed in to change notification settings - Fork 8
Added install hook. #839
Added install hook. #839
Conversation
|
💔 |
cca5ef3 to
8b7308a
Compare
hdeshev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK after fixing the $hooksService injects.
|
|
||
| constructor(protected $logger: ILogger) { | ||
| constructor(protected $logger: ILogger, | ||
| protected $hooksService: IHooksService) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
$hooksService seems unused. Why inject it all over the place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out the hook decorator needs that. It looks like we need to kill this decorator and move this feature to a method of the hook service (outside of scope for this PR).
|
💔 |
8b7308a to
0ea70d9
Compare
|
❤️ |
rosen-vladimirov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After fixing comment.
Great work
| $logger: ILogger) { | ||
| super($logger); | ||
| $logger: ILogger, | ||
| $hooksService: IHooksService) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should be tabs
0ea70d9 to
259100a
Compare
|
💔 |
|
run ci |
|
💔 |
|
run ci |
|
❤️ |
Can be useful for some scenarios like granting permissions for application via adb (automating testing process). Granting permissions require app to be deployed on device in order to work, so after-prepare hook cannot be used.