[CreateProject] allowed pretty constraint #2431
Conversation
| @@ -265,10 +265,13 @@ protected function installRootPackage(IOInterface $io, $config, $packageName, $d | |||
| $packageVersion = $requirements[0]['version']; | |||
| } | |||
|
|
|||
| $pool = new Pool($packageVersion ? 'dev' : $stability); | |||
| if ('stable' != $packageStability = VersionParser::parseStability($packageVersion)) { | |||
| $stability = $packageStability; | |||
There was a problem hiding this comment.
I would default $stability to null and then always override it with the package's stability only if the stability wasn't set explicitly.
There was a problem hiding this comment.
I don't really understand what you mean, are you saying command --stability option should win over package explicit stability?
Btw I didn't find a clean way to check whether the stability is explicitely set on the package version, I'm checking VersionParser::parseStability return, if it's not "stable" I can assume it's explicitely set, but it doesn't detects "1.1.*@stable" for example, is there a cleaner way?
Edit: hum actually something like "1.1.*@stable" shouldn't be considered here as stable is the default stability anyway.
There was a problem hiding this comment.
To parse @stable you can use: preg_match('{^[^,\s]*?@('.implode('|', array_keys(BasePackage::$stabilities)).')$}i', $packageVersion, $match)
And yes, I meant that --stability should win over package explicit stability, because it's unlikely that both are given together but if a stability is explicitly given I guess the intent is to set it to that, so keeping it that way means no BC break.
So I would say it should use the first match in this order:
- --stability if set
@..if found- parseStability
There was a problem hiding this comment.
Isn't --stabililty always set (as it has a default in the command configuration)?
Edit : well I could remove the default and take care of this at runtime, thanks for your clear explanations.
There was a problem hiding this comment.
Yup that indeed was the I would default $stability to null part in my somewhat fuzzy original post :)
|
It should be ok now, I've also updated a doc example to reflect the change. |
| @@ -57,7 +57,7 @@ protected function configure() | |||
| new InputArgument('package', InputArgument::OPTIONAL, 'Package name to be installed'), | |||
| new InputArgument('directory', InputArgument::OPTIONAL, 'Directory where the files should be created'), | |||
| new InputArgument('version', InputArgument::OPTIONAL, 'Version, will defaults to latest'), | |||
| new InputOption('stability', 's', InputOption::VALUE_REQUIRED, 'Minimum-stability allowed (unless a version is specified).', 'stable'), | |||
| new InputOption('stability', 's', InputOption::VALUE_OPTIONAL, 'Minimum-stability allowed (unless a version is specified).'), | |||
There was a problem hiding this comment.
You should leave it to VALUE_REQUIRED, it doesn't mean you have to pass a value, but that if you have --stability then it must be followed by a value, and that --stability by itself is invalid.
There was a problem hiding this comment.
definitely, fixed and squashed.
[CreateProject] allowed pretty constraint
|
Thanks! |
…onstraint [CreateProject] allowed pretty constraint
This PR allows to put pretty constrains when creating a project. It would make instructions easier for frameworks installable through composer, for example Symfony says :
"For an exact version, replace "2.3.0" with the latest Symfony version. For details, see the Symfony Installation Page"
http://symfony.com/doc/current/book/installation.html#option-1-composer
With this patch the version could be changed to "2.3.*" or "~2.3" for example, what do you think?