Skip to content
This repository was archived by the owner on Nov 14, 2019. It is now read-only.

Forward to 3.0 #198

Closed
wants to merge 4 commits into from
Closed

Forward to 3.0 #198

wants to merge 4 commits into from

Conversation

Pierstoval
Copy link
Contributor

Hey guys, now released 😄

if (!preg_match('/^2\.\d(?:\.\d{1,2})?(?:-(?:dev|BETA\d*|RC\d*))?$/i', $this->version)) {
throw new \RuntimeException('The Symfony version must be 2.N or 2.N.M (where N and M are positive integers). The special "-dev", "-BETA" and "-RC" versions are also supported.');
if (!preg_match('/^[23]\.\d(?:\.\d{1,2})?(?:-(?:dev|BETA\d*|RC\d*))?$/i', $this->version)) {
throw new \RuntimeException('The Symfony version must be 2.N or 2.N.M or 3.N or 3.N.M (where N and M are positive integers). The special "-dev", "-BETA" and "-RC" versions are also supported.');
}

if (preg_match('/^2\.\d$/', $this->version)) {
Copy link
Member

Choose a reason for hiding this comment

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

this needs an update too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

@javiereguiluz
Copy link
Member

@Pierstoval thanks for this! A new test for 3.0 version would be great. You just need to add a new item to this data provider: https://github.com/symfony/symfony-installer/blob/master/tests/Symfony/Installer/Tests/IntegrationTest.php#L96

@Pierstoval
Copy link
Contributor Author

@javiereguiluz I added the test but it'd need more integration to make it succeed, because of Symfony3 different structure, so it'll need more updates on the tests :)

@javiereguiluz
Copy link
Member

@Pierstoval and that's why the installer didn't have support for Symfony 3 yet. It's not a matter of changing just a number :)

@Pierstoval
Copy link
Contributor Author

I'm on it ;)

@Pierstoval
Copy link
Contributor Author

Done it!

Just, there's still a problem: the composer.lock stores a version code as dev-master, whereas it should be 3.0.0-BETA1 as well as any other beta version (this is why the test fails)

Plus, I don't know when it'll be available (I don't know this info about the release process) but the http://symfony.com/versions.json should be updated in order to show the state of the 3.0 branch, but I don't know if we need a proper release for this or if the beta is sufficient.

@Pierstoval
Copy link
Contributor Author

If needed, I can patch a workaround for the composer.lock version problem, but I don't know Symfony policy about 3.0 releases.
Anyway it would make the tests succeed and a new release of the installer could be done.

*/
protected function isSymfony3()
{
return strpos($this->version, '3') === 0;
Copy link
Member

Choose a reason for hiding this comment

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

return '3' === $this->version[0];

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks! I didn't know this syntax would be fully compatible :)

@javiereguiluz
Copy link
Member

I've tested this PR and everything works fine for me. I'll merge it soon unless someone raises a valid concern.

@Pierstoval I'd like to do just a minor change:

-                "    * Change your current directory to <comment>%s</comment>\n\n", $this->projectDir
+                "    * Change your current directory to <comment>%s</comment>\n", $this->projectDir

This change prevents the ugly double blank line in the command output:

excessive_blank_lines

@Pierstoval
Copy link
Contributor Author

Done in eba3537 😄

@javiereguiluz
Copy link
Member

@Pierstoval thanks for working on this. I've merged the changes and I'll release a new version of the installer soon, so we can test everything well and it's ready for the big Symfony 2.8 and 3.0 launch next week. Thanks!

@Pierstoval Pierstoval deleted the to_3.0 branch November 26, 2015 21:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants