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

add install checker #3023

Merged
merged 18 commits into from
Aug 24, 2016
Merged

add install checker #3023

merged 18 commits into from
Aug 24, 2016

Conversation

craigh
Copy link
Member

@craigh craigh commented Aug 22, 2016

Q A
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Fixed tickets -
Refs tickets #3025, #2235, #3012
License MIT
Changelog updated yes

Description

Add Symfony Installer's requirements check to our own installation and upgrade processes. Update and improve install and upgrade docs. Eliminate the use of system constants to indicate currently installed version and replace with permanently written parameter.

use Symfony\Component\Console\Output\OutputInterface;
use Zikula\Bundle\CoreBundle\YamlDumper;

class UpgradeResetCommand extends ContainerAwareCommand
Copy link
Contributor

Choose a reason for hiding this comment

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

So this is only here for Travis, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see that it would be useful for anything else, no.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be more appropriate not to add this command but update the version using a shell command inside the travis.yml file.

Copy link
Member Author

Choose a reason for hiding this comment

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

PR welcome

@craigh craigh added this to the 1.4.3 milestone Aug 23, 2016
@craigh
Copy link
Member Author

craigh commented Aug 23, 2016

  • note to self: remove the src/zikulaCheck.php file

@craigh craigh changed the title [WIP] add install checker add install checker Aug 24, 2016
@craigh
Copy link
Member Author

craigh commented Aug 24, 2016

This is finished. please review.

@@ -79,25 +76,23 @@ public function initPhp()

public function requirementsMet(ContainerInterface $container)
{
// several other requirements are checked before Symfony is loaded.
Copy link
Member

Choose a reason for hiding this comment

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

So below only Zikula-specific checks remain, right?

@Guite
Copy link
Member

Guite commented Aug 24, 2016

Nice addition 😄

function_exists('mb_get_info'),
'mbstring is not installed in PHP. Zikula cannot install or upgrade without this extension.',
'Install and enable the <strong>mbstring</strong> extension'
);
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be removed as well.

@cmfcmf
Copy link
Contributor

cmfcmf commented Aug 24, 2016

I've replaced the upgrade command now with the sed command. I accidentally directly pushed to your branch 😕 Currently waiting for the builds to start, finish and succeed :)

@cmfcmf
Copy link
Contributor

cmfcmf commented Aug 24, 2016

Currently waiting for the builds to start, finish and succeed :)

It's working 🎉 Only thing left to do is to remove the command file you introduced.

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

Successfully merging this pull request may close these issues.

3 participants