Skip to content
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

Make installation more robust #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

helhum
Copy link

@helhum helhum commented Aug 22, 2016

  • Remove autoload.php generation
  • Make using the puli/cli in the same project work

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).


return;
}

Copy link
Contributor

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 ?

Copy link
Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it :)

@helhum
Copy link
Author

helhum commented Aug 23, 2016

I'm now investigating why the build fails

* Remove autoload.php generation
* Make using the puli/cli in the same project work
Copy link
Member

@webmozart webmozart left a 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'])) {
Copy link
Member

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants