added question helper compatibility alongside the dialog in CreateSit… #816

Dunduro · 2017-05-17T11:03:05Z

I am targeting this branch, because this is BC.

Changelog

### Fixed
- added support for both `QuestionHelper` and `DialogHelper` in `CreateSiteCommand` for Symfony 2.3 and 3.x compatibility

Subject

Symfony 3 intergration

Dunduro · 2017-05-17T11:47:59Z

heej i rebased and fixed the structrual stuf i had done wrong, how do i get the test to run again?

#magic?!?!

greg0ire · 2017-05-17T12:09:57Z

Command/CreateSiteCommand.php

    }

    /**
     * {@inheritdoc}
     */
    public function execute(InputInterface $input, OutputInterface $output)
    {
-        $dialog = $this->getHelperSet()->get('dialog');
+        if (!$this->getHelperSet()->has('question')) {


Please remove the ! and swap lines 60 and 62

greg0ire · 2017-05-17T12:10:52Z

Command/CreateSiteCommand.php

+            $helper = $this->getHelperSet()->get('dialog');
+        } else {
+            $helper = $this->getHelperSet()->get('question');
+        }


Please provide NEXT_MAJOR instructions indicating what to do when dropping support for old versions of sf

changed the if statements around,

is there a specific best practice for adding these instructions?
and where can i read up on that?

You can find it here:
https://github.com/sonata-project/SonataPageBundle/blob/3.x/CONTRIBUTING.md

greg0ire · 2017-05-17T12:11:21Z

Command/CreateSiteCommand.php

+        } else {
+            $question = new \Symfony\Component\Console\Question\ConfirmationQuestion('Confirm site creation (Y/N)', false, '/^(y|j)/i');
+            $confirmation = $helper->ask($input, $output, $question);
+        }


greg0ire · 2017-05-17T12:27:06Z

Please associate your webengine.nl email address with your Github account, or change the email in your commits to an address already associated with it.

greg0ire · 2017-05-17T12:29:23Z

heej i rebased and fixed the structrual stuf i had done wrong, how do i get the test to run again? #magic?!?!

I cancelled the second last build so that the last build would run. A build is run each time you push ;)

greg0ire · 2017-05-17T12:46:08Z

Command/CreateSiteCommand.php


-        if ($input->getOption('no-confirmation') || $dialog->askConfirmation($output, 'Confirm site creation ?', false)) {
+        if ($this->getHelperSet()->has('question')) {
+            $question = new \Symfony\Component\Console\Question\ConfirmationQuestion('Confirm site creation (Y/N)', false, '/^(y|j)/i');


Please linebreak this

greg0ire · 2017-05-17T12:46:21Z

Command/CreateSiteCommand.php

+            $question = new \Symfony\Component\Console\Question\ConfirmationQuestion('Confirm site creation (Y/N)', false, '/^(y|j)/i');
+            $confirmation = $helper->ask($input, $output, $question);
+        } else {
+            $confirmation = $input->getOption('no-confirmation') || $helper->askConfirmation($output, 'Confirm site creation ?', false);


Linebreak this too please.

greg0ire · 2017-05-17T14:05:56Z

Command/CreateSiteCommand.php

    }

    /**
     * {@inheritdoc}
     */
    public function execute(InputInterface $input, OutputInterface $output)
    {
-        $dialog = $this->getHelperSet()->get('dialog');
+        /*
+         * NEXT_MAJOR: do remove if statment and use the question helper only (true value of if) when dropping sf < 2.5


- do remove if statment and use the question helper only (true value of if) when dropping sf < 2.5 + remove if statement and use the question helper only (true value of if) when dropping sf < 2.5

greg0ire · 2017-05-17T14:06:55Z

Command/CreateSiteCommand.php

-        if ($input->getOption('no-confirmation') || $dialog->askConfirmation($output, 'Confirm site creation ?', false)) {
+        /*
+         * NEXT_MAJOR: do remove if statment and use the question helper only (true value of if) when dropping sf < 2.5
+         */


Same here, and after that, good to merge :)

oke fixed that

greg0ire · 2017-05-17T14:45:02Z

Command/CreateSiteCommand.php

    }

    /**
     * {@inheritdoc}
     */
    public function execute(InputInterface $input, OutputInterface $output)
    {
-        $dialog = $this->getHelperSet()->get('dialog');
+        /*
+         * NEXT_MAJOR: remove if statment and use the question helper only (true value of if) when dropping sf < 2.5


- statment + statement

greg0ire · 2017-05-17T17:30:09Z

@sonata-project/contributors please review

core23 · 2017-05-17T17:53:12Z

Command/CreateSiteCommand.php

+        } else {
+            $confirmation = $input->getOption('no-confirmation') || $helper->askConfirmation(
+                $output,
+                'Confirm site creation ?',


No whitespace infront of the question mark please

Ping @Dunduro

core23 · 2017-05-17T17:53:33Z

Command/CreateSiteCommand.php

+            $question = new \Symfony\Component\Console\Question\ConfirmationQuestion(
+                'Confirm site creation (Y/N)',
+                false,
+                '/^(y|j)/i'


Why did you allow the letter j?

Wel thats a funny story but i was looking the symfony 3 explanation and it had it as an example. But thought it made sense since symfony is in essence a multi langual platform, and in dutch for instance is 'ja'. But if ya think it shouldnt be there i dont mind taking it away

If you keep it, please add o for french, s for spanish, i for hungarian, h for Klingon and so on

Haha good point :p i'll get it fixed tomorrow

oke fixed now

…eCommand

greg0ire · 2017-05-19T14:54:28Z

Thanks @Dunduro !

…ommand (sonata-project#816)

SonataCI added the review required label May 17, 2017

Dunduro force-pushed the 3.x branch from c364d7e to 30b84aa Compare May 17, 2017 11:45

Dunduro force-pushed the 3.x branch from 30b84aa to 527016c Compare May 17, 2017 11:52

greg0ire requested changes May 17, 2017

View reviewed changes

Dunduro force-pushed the 3.x branch from 527016c to e786b8c Compare May 17, 2017 12:17

OskarStark added the patch label May 17, 2017

Dunduro force-pushed the 3.x branch from e786b8c to 2ed5eaa Compare May 17, 2017 12:37

greg0ire reviewed May 17, 2017

View reviewed changes

Dunduro force-pushed the 3.x branch 2 times, most recently from 0c82359 to 7693337 Compare May 17, 2017 13:12

greg0ire approved these changes May 17, 2017

View reviewed changes

greg0ire reviewed May 17, 2017

View reviewed changes

greg0ire requested changes May 17, 2017

View reviewed changes

Dunduro force-pushed the 3.x branch from 7693337 to 057ccb0 Compare May 17, 2017 14:30

greg0ire reviewed May 17, 2017

View reviewed changes

Dunduro force-pushed the 3.x branch from 057ccb0 to 5d312be Compare May 17, 2017 14:49

greg0ire approved these changes May 17, 2017

View reviewed changes

core23 reviewed May 17, 2017

View reviewed changes

core23 requested changes May 17, 2017

View reviewed changes

Dunduro force-pushed the 3.x branch 2 times, most recently from 143a5aa to 7ea64e8 Compare May 18, 2017 08:31

added question helper compatibility alongside the dialog in CreateSit…

3dc62c8

…eCommand

Dunduro force-pushed the 3.x branch from 7ea64e8 to 3dc62c8 Compare May 19, 2017 13:57

core23 approved these changes May 19, 2017

View reviewed changes

greg0ire merged commit 4c19123 into sonata-project:3.x May 19, 2017

OskarStark removed the review required label May 22, 2017

jayesbe pushed a commit to Indinuity/SonataPageBundle that referenced this pull request Aug 21, 2017

Add question helper compatibility alongside the dialog in CreateSiteC…

23fff3e

…ommand (sonata-project#816)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added question helper compatibility alongside the dialog in CreateSit… #816

added question helper compatibility alongside the dialog in CreateSit… #816

Dunduro commented May 17, 2017 •

edited by OskarStark

Loading

Dunduro commented May 17, 2017 •

edited

Loading

greg0ire May 17, 2017

greg0ire May 17, 2017

Dunduro May 17, 2017

OskarStark May 17, 2017

Dunduro May 17, 2017

greg0ire May 17, 2017

greg0ire commented May 17, 2017

greg0ire commented May 17, 2017

greg0ire May 17, 2017

greg0ire May 17, 2017

greg0ire May 17, 2017 •

edited

Loading

greg0ire May 17, 2017

Dunduro May 17, 2017

greg0ire May 17, 2017

greg0ire commented May 17, 2017

core23 May 17, 2017

greg0ire May 18, 2017

core23 May 17, 2017

Dunduro May 17, 2017

greg0ire May 17, 2017

Dunduro May 17, 2017

Dunduro May 18, 2017

greg0ire commented May 19, 2017

added question helper compatibility alongside the dialog in CreateSit… #816

added question helper compatibility alongside the dialog in CreateSit… #816

Conversation

Dunduro commented May 17, 2017 • edited by OskarStark Loading

Changelog

Subject

Dunduro commented May 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg0ire commented May 17, 2017

greg0ire commented May 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg0ire May 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg0ire commented May 17, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greg0ire commented May 19, 2017

Dunduro commented May 17, 2017 •

edited by OskarStark

Loading

Dunduro commented May 17, 2017 •

edited

Loading

greg0ire May 17, 2017 •

edited

Loading