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

Allow last LTS version to be installed. #105

Closed
wants to merge 2 commits into from
Closed

Allow last LTS version to be installed. #105

wants to merge 2 commits into from

Conversation

94noni
Copy link
Contributor

@94noni 94noni commented Feb 20, 2015

Hi,

I just wanted to add the possibility to install the latest Symfony LTS version directly via the installer (new command).

Docs PR symfony/symfony-docs#5032

Feedbacks welcome :)

}

// Get the Symfony versions list (http://symfony.com/versions.json)
$versionsList = $this->getVersionsList();
Copy link
Member

Choose a reason for hiding this comment

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

this is no loading the version list in cases where it is not needed (passing an exact version), which is wrong. The HTTP request should be done only when needed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes of course, going to fetch+cache them

Copy link
Member

Choose a reason for hiding this comment

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

Well, this is still only necessary if the user doesn't pass a version number in the 2.M.N format (to either fetch the latest patch version, the LTS version or the latest stable version).

Copy link
Member

Choose a reason for hiding this comment

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

@94noni what you should do is move this call inside the if (self::VERSION_LTS === $this->version) { conditional (and add it again in the place where you removed it). The call won't be done twice anyway because the JSON gives you the exact version for the LTS, not the branch

Copy link
Contributor

Choose a reason for hiding this comment

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

Unless symfony.com plans to propose a proper "standard" download link for the LTS version, this behavior seems correct to me.

@javiereguiluz
Copy link
Member

If you want, we can make lts version name to work on symfony.com. We already do this for latest version name and it would be really easy to do. This way we could simplify a lot the code of this PR. What do you think?

@94noni
Copy link
Contributor Author

94noni commented Feb 20, 2015

@javiereguiluz exactly what i asked for @stof

can you please just explain me this:
http://symfony.com/download?v=Symfony_Standard_Vendors_latest => can be resolved
http://symfony.com/download?v=Symfony_Standard_Vendors_lts => can not be resolved
it will be easier (and no more query call for json for the lts) if it is resolved correctly :)

@Pierstoval
Copy link
Contributor

This could reduce this PR to two lines of code, I think it's worth the cost 😃

@javiereguiluz
Copy link
Member

OK. The lts version is already supported in symfony.com. But we must wait for the related pull request to get approved and deployed in the production website. I'll keep you posted.

@xabbuh
Copy link
Member

xabbuh commented Feb 20, 2015

@javiereguiluz Will that be the latest LTS once Symfony 2.7 is released?

@javiereguiluz
Copy link
Member

@xabbuh yes. It will always point to the most recent LTS version, whichever it is.

@stof
Copy link
Member

stof commented Feb 20, 2015

@94noni can you update the PR so that it simply handle lts in the same way than latest (which should indeed be a 2 lines patch), even if the archive is not yet in production (it only means we have to wait before merging, but it does not prevent to have the patch ready)

@94noni
Copy link
Contributor Author

94noni commented Feb 20, 2015

Thank all of you, waiting your go to change this (and PR on the related symfony documentation)
Edit: ok lets do that, I reverted the code

@stof
Copy link
Member

stof commented Feb 20, 2015

Can you revert the other changes being done here to really keep it a 2-line patch (well, maybe 4 because of formatting)

@94noni
Copy link
Contributor Author

94noni commented Feb 20, 2015

@stof PR finalized, much cleaner with the symfony.com lts version supported now

@stof
Copy link
Member

stof commented Feb 20, 2015

👍

@94noni
Copy link
Contributor Author

94noni commented Feb 20, 2015

Just forget the read me. Updated now

@javiereguiluz
Copy link
Member

The special lts version name is already available in production:

http://symfony.com/download?v=Symfony_Standard_Vendors_lts.zip
http://symfony.com/download?v=Symfony_Standard_Vendors_lts.tgz

@stof
Copy link
Member

stof commented Feb 27, 2015

👍 then

@94noni
Copy link
Contributor Author

94noni commented Mar 1, 2015

Cool :)
And is it ok for the symfony/symfony-docs#5032 as well?

@javiereguiluz
Copy link
Member

Thanks @94noni! It took me a lot of time to merge this, but it's finally done :)

weaverryan pushed a commit to symfony/symfony-docs that referenced this pull request Mar 14, 2015
weaverryan added a commit to symfony/symfony-docs that referenced this pull request Mar 14, 2015
This PR was submitted for the 2.6 branch but it was merged into the 2.3 branch instead (closes #5032).

Discussion
----------

Minor improvement for symfony-installer with LTS

Minor improvement for symfony-installer with LTS

based on the symfony-installer PR symfony/symfony-installer#105

Commits
-------

18c08f3 Better wording
8e41e73 Minor improvement for symfony-installer with LTS
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants