Improve config options #1366
Improve config options #1366
Conversation
|
Somewhat off-topic, but I've just seen that composer.json still has |
|
@LycheeOrg/reviewers This PR should be reviewed because it contains some new config options that are already listed in the docs. Could you please re-review it? |
|
@nagmat84 I added the session store option back. You still need to approve because you requested changes :) |
composer.json
Outdated
| @@ -1,10 +1,20 @@ | |||
| { | |||
| "name": "lycheeorg/lychee-laravel", | |||
| "name": "lycheeorg/lychee", | |||
There was a problem hiding this comment.
Just something which came to my mind again. The Github URL is LycheeOrg/Lychee with three capital letters. I don't know if this is relevant for anything at all or in case "something" interprets this entry, that "something" does so in a case-sensitive manner.
Anyway, I just re-remembered that we are astonishingly inconsistent about our capitalization: not only here, but also in the frontend, etc.
There was a problem hiding this comment.
Can we ignore it or should I update it?
There was a problem hiding this comment.
I would go for lychee-org/lychee because our other packages on packagist.org are under that common name
There was a problem hiding this comment.
It's fairly common for package names etc to be lower case while the project itself has capitalisation. By no means universal, but common. Especially with package managers.
There was a problem hiding this comment.
| "name": "lycheeorg/lychee", | |
| "name": "lychee-org/lychee", |
There was a problem hiding this comment.
Yes, @ildyria was. That's another inconsistency we have and when I spotted it, it had been to late.
I guess we all agree that it would be awesome to use the same spelling of "Lychee Org" everywhere, i.e. here on Github and on Packagist. I don't know if it is possible to rename our packages and create some kind of "redirection" on Packagist from the old name to the new name such that we don't break any dependencies.
There was a problem hiding this comment.
"require": {
"php": "^8.0",
"ext-bcmath": "*",
"ext-ctype": "*",
"ext-exif": "*",
"ext-fileinfo": "*",
"ext-gd": "*",
"ext-json": "*",
"ext-mbstring": "*",
"ext-openssl": "*",
"ext-pdo": "*",
"ext-tokenizer": "*",
"ext-xml": "*",
"bepsvpt/secure-headers": "^7.1",
"darkghosthunter/larapass": "dev-LycheeSpecial",
"doctrine/dbal": "^3.1",
"fideloper/proxy": "^4.3",
"geocoder-php/cache-provider": "^4.3",
"geocoder-php/nominatim-provider": "^5.5",
"laravel/framework": "^8.83.14",
"livewire/livewire": "^2.7",
"lychee-org/nestedset": "^6",
"lychee-org/php-exif": "^0.7.11",
"maennchen/zipstream-php": "^2.1",
"php-ffmpeg/php-ffmpeg": "^1.0",
"php-http/guzzle7-adapter": "^1.0",
"php-http/message": "^1.12",
"spatie/guzzle-rate-limiter-middleware": "^2.0",
"spatie/laravel-feed": "^4.0",
"spatie/laravel-image-optimizer": "^1.6.2",
"symfony/cache": "^v6.0.0",
"whichbrowser/parser": "^2.0"
},
"require-dev": {
"ext-imagick": "*",
"ext-posix": "*",
"ext-zip": "*",
"barryvdh/laravel-debugbar": "^3.6",
"barryvdh/laravel-ide-helper": "^2.10",
"filp/whoops": "^2.5",
"friendsofphp/php-cs-fixer": "^3.3",
"itsgoingd/clockwork": "^5.0",
"laravel/homestead": "^v13.2.1",
"lychee-org/phpstan-lychee": "dev-master",
"mockery/mockery": "^1.5",
"nunomaduro/collision": "^5.0",
"nunomaduro/larastan": "^1.0",
"php-parallel-lint/php-parallel-lint": "^1.3",
"phpunit/phpunit": "^9"
},camelCase does not seem standard there.
There was a problem hiding this comment.
If we take example on https://github.com/FriendsOfPHP it should be "lycheeorg"
There was a problem hiding this comment.
I was just curious :). I'm slightly in favour of lycheeorg but it's not a major issue for me.
FWIW, we're lycheeorg on Docker Hub. That one was probably me.
There was a problem hiding this comment.
Found in a SO post: composer/packagist#47
Which raises the question of why it was working in the first place.
Answer: Because the packages we publish are in lychee-org form https://github.com/LycheeOrg/laravel-nestedset/blob/v6/composer.json#L2
So I'd use lychee-org for now until/unless we want to change properly.
|
@qwerty287 There have been some changes with Laravel 9. Or we can merge this now and see what is conflicting with #1469 :) |
|
I'd merge this first and then look at #1469. I can see what I can do regarding conflicts there, but I'd merge this first. |
|
I guess we are good to merge this one? |
|
From my side yes :) |
HASHING_ALGORITHM)