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

Minor fix: Fixed Module generation with Controller option=YES #115

Merged
merged 3 commits into from
Jul 3, 2014

Conversation

itsekhmistro
Copy link

Fixed parameters for Controller template in ModuleGenerator.
Fixed creating of Directory structure in ModuleGenerator.

@jmolivas
Copy link
Member

jmolivas commented Jul 3, 2014

Thanks for the PR @bearsite and for noticing it was not generating the controller

May you change the code to something like this same as in Controller generation https://github.com/hechoendrupal/DrupalAppConsole/blob/master/src/Command/GeneratorControllerCommand.php#L118

I will add comments on the suggested lines

$class_machine_name = 'default_controller';
$parameters = array(
'class_name' => $class_name,
// 'services' => $services,
Copy link
Member

Choose a reason for hiding this comment

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

Remove comment line

@itsekhmistro
Copy link
Author

Thank you @jmolivas. Please check if I applied your comments correctly.

@jmolivas
Copy link
Member

jmolivas commented Jul 3, 2014

Already tested and works fine thanks I will go ahead and merge the PR

jmolivas added a commit that referenced this pull request Jul 3, 2014
Minor fix: Fixed Module generation with Controller option=YES
@jmolivas jmolivas merged commit 49f5b4c into hechoendrupal:master Jul 3, 2014
@dmouse
Copy link
Member

dmouse commented Jul 3, 2014

👍 @bearsite

@jmolivas
Copy link
Member

jmolivas commented Jul 3, 2014

Thanks for the contribution @bearsite 👍

@itsekhmistro
Copy link
Author

Thank you @jmolivas , @dmouse

g3r4 pushed a commit to g3r4/drupal-console that referenced this pull request Jul 19, 2017
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