Checking that symfony installer is up-to-date #221
Conversation
|
Hehe, we proposed the same feature at the same time 😆 (#221) |
| */ | ||
| class Application extends ConsoleApplication | ||
| { | ||
| const VERSIONS_URL = 'http://get.sensiolabs.org/symfony.version'; |
There was a problem hiding this comment.
I suggest using get.symfony.com instead (this is the Symfony installer after all), as it also works (and even with https)
There was a problem hiding this comment.
will change to get.symfony.com but we still don't use https here
https://github.com/symfony/symfony-installer/blob/master/src/Symfony/Installer/SelfUpdateCommand.php#L125 ?
Is there any reason
There was a problem hiding this comment.
mostly because the last time we worked on the SelfUpdateCommand, SSL was not yet available
There was a problem hiding this comment.
updated to https - thanks for claryifing
|
@Pierstoval yes - minutes away! :D 2 solutions are still better then no solution! |
| { | ||
| $commandName = $this->getCommandName($input); | ||
|
|
||
| if ($this->isInPharMode() && $commandName != 'self-update' && $commandName != 'selfupdate') { |
There was a problem hiding this comment.
- if ($this->isInPharMode() && $commandName != 'self-update' && $commandName != 'selfupdate') {
+ if ($this->isInPharMode() && !in_array($commandName, array('self-update', 'selfupdate'), true) {There was a problem hiding this comment.
changed - thx for review
| { | ||
| $commandName = $this->getCommandName($input); | ||
|
|
||
| if ($this->isInPharMode() && !in_array($commandName, array('self-update', 'selfupdate'), true)) { |
There was a problem hiding this comment.
Just asking: shouldn't we also avoid to call this command for list, help and the other common actions? Maybe we could replace the !in_array($commandName, array(...)) by in_array($commandName, array('new', 'demo')) ?
There was a problem hiding this comment.
At first I was 👍 You are right that those are mostly needed on new and demo, but after some considerations:
What if we change some command names/parameters and user will get old version on help (different than e.g. on symfony.com site) - that could make some problems I think?
Maybe we should still check version there and set notification but not show it as warning?
There was a problem hiding this comment.
IMHO, commands' arguments and options are not likely to change very often, and the help command will still be valid for the current installed version. If someone does not have a new option, I think he'll figure out his installer is outdated.
So 👍 about Javier's suggestion.
|
@jzawadzki thanks for this feature! (and thanks to @Pierstoval too for working on this feature in another pull request!) |
Hi,
regarding our discussion in #220 and #136 I'm sending first draft of checking if installer is up-to-date.
I have based my solution of how and when composer is checking it.
Please comment and tell if this solution is ok and what should I change to make it better!
Thanks,
JZ