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

@helhum helhum force-pushed the cleanup-installation branch from 8732077 to ddd31e7 Compare August 22, 2016 21:39

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 helhum force-pushed the cleanup-installation branch from ddd31e7 to ff0f3d5 Compare August 23, 2016 10:32
@helhum
Copy link
Author

helhum commented Aug 23, 2016

I'm now investigating why the build fails

@helhum helhum force-pushed the cleanup-installation branch from ff0f3d5 to 8725599 Compare August 23, 2016 11:21
* Remove autoload.php generation
* Make using the puli/cli in the same project work
@helhum helhum force-pushed the cleanup-installation branch from 8725599 to 069abe3 Compare August 23, 2016 11:22
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
Development

Successfully merging this pull request may close these issues.

3 participants