detect dev for install command #1413
Conversation
src/Entity/Package.php
Outdated
| $command = 'require'; | ||
|
|
||
| /** @var Version $version */ | ||
| $version = $this->getVersions()->first(); |
There was a problem hiding this comment.
not sure which version to use ...
There was a problem hiding this comment.
The view_package template already receive a version variable (which can be null). this is the version that should be used for the install command IMO (which would be done by keeping the command in the template as it is done right now, but adding a {% if version and version.hasDevTag %} --dev{% endif %})
There was a problem hiding this comment.
yea, i know that moving it from the template to PHP feels like the more complex solution at first, but i tend to kill every if in twig that i can replace with sth that is easily unit tested.
but sure, that's a debatable design decision
There was a problem hiding this comment.
Twig already has the right version to use passed to it by the controller. The Package entity does not have it. So moving this to the entity itself looks like the bad decision IMO.
There was a problem hiding this comment.
wasn't aware of that controller code to sort out the most relevant version - thanks for that pointer!
will give it another thought later 👍
There was a problem hiding this comment.
IMO this function should just accept a version as argument, and then the template can pass it in.
There was a problem hiding this comment.
easy solution, true - done
chr-hertel
force-pushed
the
install-command-dev
branch
from
23a56fb to
943a092
Compare
src/Entity/Version.php
Outdated
| @@ -635,6 +635,13 @@ public function getTags(): Collection | |||
| return $this->tags; | |||
| } | |||
|
|
|||
| public function hasDevTag(): bool | |||
| { | |||
| return [] !== array_filter($this->getTags()->toArray(), static function (Tag $tag) { | |||
There was a problem hiding this comment.
looping over the tags and stopping when we find a dev tag would be more efficient than filtering the whole list all the time.
src/Entity/Package.php
Outdated
| $command = 'require'; | ||
|
|
||
| /** @var Version $version */ | ||
| $version = $this->getVersions()->first(); |
There was a problem hiding this comment.
The view_package template already receive a version variable (which can be null). this is the version that should be used for the install command IMO (which would be done by keeping the command in the template as it is done right now, but adding a {% if version and version.hasDevTag %} --dev{% endif %})
chr-hertel
force-pushed
the
install-command-dev
branch
from
943a092 to
4a6ad77
Compare
chr-hertel
force-pushed
the
install-command-dev
branch
from
cd4cbab to
020a71d
Compare
|
Thanks! |
| @@ -46,7 +46,7 @@ | |||
|
|
|||
| <div class="row"> | |||
| <div class="col-md-8"> | |||
| <p class="requireme"><i class="glyphicon glyphicon-save"></i> <input type="text" readonly="readonly" value="composer {% if package.type == 'project' %}create-project{% else %}require{% endif %} {{ "#{package.vendor}/#{package.packageName}" }}" /></p> | |||
| <p class="requireme"><i class="glyphicon glyphicon-save"></i> <input type="text" readonly="readonly" value="{{ package.installCommand(expandedVersion) }}" /></p> | |||
There was a problem hiding this comment.
shouldn't this use version instead of expandedVersion to be consistent with the sidebar showing the support action.
There was a problem hiding this comment.
Yes true it should be version, I'll fix that.
Fixes #1412