-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update issues with valet on arch linux #141
base: master
Are you sure you want to change the base?
Conversation
Fixed the default package version for pacman to ''; Pacman does have packages of the form php{version}-{extension}. so i chanded the default version of pacman to php-{extension} NB: This occurs only when the packageManager used is pacman.
…e is missing Sometimes, valet install/uninstall will throw some file not found errors when you try installing valet in computer with no previous config files or config file absent
… inexistent old path
If u tried installing nginx in a situation where ~/.config/valet/Nginx doesn't exist, it'll throw a warning saying the path/file (previous configured) doesn't exist
With the previous changes made the prevents valet from getting configs from inexitent config files, i had the add a default custom domain while resecuring nginx configs
from the list of php extensions required by php-fpm, the only one installable on its own is php-gd (pacman package managers). I made sure to ignore the orders which caused an issue while downloading extensions. The others come with php by default
…ig file doesn't exist Sometimes, valet's config file may be missing and will be suitable to add a default domain ('test') in case the config domain doesn't exists
…compatibility Recent versions of ```ca-certificates``` are installed in ```/usr/share/ca-certificates``` whereas the path specified in valet is ```/usr/local/share/ca-certificates```. the previous path wasn't removed. but a check is done first to check if prevoious path exists before using recent path
foreach (self::COMMON_EXTENSIONS as $ext) { | ||
$extArray[] = "{$extensionPrefix}{$ext}"; | ||
|
||
if (($this->pm instanceof \Valet\PackageManagers\Pacman)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, instead of adding if else statement here, can we relay on methods from PackageManager?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, i have to leave it as such since php
for pacman automatically ships in with all extensions (except php-apache
, php-apcu
, php-cgi
, php-dblib
, php-embed
, php-enchant
,php-geoip
, php-gd
which am not sure if they're all php extensions).
The only package needed by valet here is php-gd
. Don't know how it works for other package managers though.
Please enlighten me on how i can takle this problem
There are some feedbacks on your code, and I am still reviewing it, but can you make sure that your changes does not break unit tests? |
…rsioned packages I create a new method on the PackageManager Contract which determines if a given package manager supports package versionning and implemented the method on all Package managers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made some changes regarding 2 issues outlined out of 3.
I don’t know If i did something wrongly but it’s been months am waiting for this to be reviewed. |
@Stephan-MC I apologise for not reviewing this for a quite long time, but my schedule is messed up and due to that reason I couldn't spare more time for this repository. I will definitely find some time off from my work and will review it. |
php-gd
where valet tries to install every thing on arch which may cause issues)php{version}-{ext}
).