Open drzraf opened 2 years ago
Hallo @drzraf! 👋🏻
The Gmail address you used in the commit is not registered here on GitHub.
The Gmail address you used in the commit is not registered here on GitHub.
Sounds like GH decided to drop my +foobar
delimiter from my email thus "breaking" the reference of all my commit. Does it cause any actual trouble?
Does it cause any actual trouble?
No. Not at all. This is just one of my policies.
Hi @drzraf! Thank you for your pull request. I like the direction in which this is going. Here are some general considerations:
People may use this Composer installer for multiple (unrelated) packages. So we need to find a way to target specific packages/dist URLs in extra.private-composer-installer
.
What I have in mind is adding a hash suffix #config-1
to the dist URL that we may use to target a specific configuration in extra.private-composer-installer.configs
(for lack of a better name). This way multiple packages (from the same vendor) could reuse the same config. I would keep a config
generic to allow for future additions that may be unrelated to an indirection feature.
Example:
{
"extra": {
"private-composer-installer": {
"configs": {
"config-1": {
"indirection": {
"http": { "method": "POST"},
"parse": { "format": "json", "key": "download_link" }
}
}
}
}
}
}
key
location in the JSON payload using an object structure. I would go with a simple dot-notation string e.g. foo.bar.0.download_url
to specify it.People may use this Composer installer for multiple (unrelated) packages. So we need to find a way to target specific packages/dist URLs in extra.private-composer-in
But if is that already the case if the extra property lives at the package-level ?
* I don't like the way you provide the `key` location in the JSON payload using an object structure. I would go with a simple dot-notation string e.g. `foo.bar.0.download_url` to specify it.
Neither do I, but would you tolerate a dependency on https://symfony.com/doc/current/components/property_access.html ? Or prefer a custom accessor based on a dot notation ? Or something more powerful based on xpath or json-path?
Keep in mind that dist URLs without any placeholders should be ignored by this plugin to not alter unrelated packages. An indirection call must only be made if an indirection is actually configured (I think that's not the case right now in your code).
But an URL without placeholder may still legitimately make use of this feature, isn't?
Fix #22 by allowing a package URL indirection to take place.
PR supports:
May support out of the box (or with further tweaks):
User-Agent
, not tested, see$event->HttpDownloader
)$event->HttpDownloader
. Standard mechanism apply. May be possible out of the boxNot supported (but could be implemented)
Not supported:
curl_setopt($curlHandle, CURLOPT_FOLLOWLOCATION, false)
Configuration:
This is the
extra
property (at the package-level):For nested keys, it could be
"key": {"foo": {"bar": "download_link"}}
A workflow is like:
activate_license
for a given website (only once)/?edd_action=activate_license&license=0123456789abcdf&name=package-slug&url=https%3A//my.domain.tld
dist.url
tohttps://package-provider.tld/edd_url?edd_action=get_version&license=0123456789abcdf&name=foobar&url=https%3A//my.domain.tld
(@junaidbhura? In case you think this PR, if generic enough, could make composer-wp-pro-plugins somehow redundant?)