-
Notifications
You must be signed in to change notification settings - Fork 19
Make installation more robust #49
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
8732077
to
ddd31e7
Compare
|
||
return; | ||
} | ||
|
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.
Why did you remove the try catch ?
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.
Hope the new version makes that more clear. The code executed does not throw a PuliRunnerException any more, thus the try/catch does not make sense any more. The exceptions that might be triggered really are fatals (mainly programming mistakes in the plugin), which should bubble up.
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.
Got it :)
ddd31e7
to
ff0f3d5
Compare
I'm now investigating why the build fails |
ff0f3d5
to
8725599
Compare
* Remove autoload.php generation * Make using the puli/cli in the same project work
8725599
to
069abe3
Compare
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.
Looks good to me!
} | ||
switch ($key) { | ||
case 'factory.in.file': | ||
if (empty($config) || empty($config['config']['factory']['in']['file'])) { |
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.
Isn't the empty($config)
implied by the right-hand-side of the condition?
This is achieved by reading the puli.json directly for
the factory file and class name configuration (instead of running the puli cli, which is not ready yet
if it is installed in the same project).