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

Fix "find config" #2314

Merged
merged 1 commit into from Nov 6, 2017

Conversation

Projects
None yet
4 participants
@ozh
Member

ozh commented Nov 3, 2017

realpath() seems to have many caveats depending on the platform (Win/Linux/BSD) and various settings (namely, open_basedir restrictions)

Ping @boldcity and @KabataMacharia, if you could test this patch on your platform it would be very great. Currently works on Windows and Ubuntu

Fix "find config".
realpath() seems to have many caveats depending on the platform
@LeoColomb

This comment has been minimized.

Show comment
Hide comment
@LeoColomb

LeoColomb Nov 3, 2017

Member

@ozh It's already fixed with 4cb866a, isn't it?

Member

LeoColomb commented Nov 3, 2017

@ozh It's already fixed with 4cb866a, isn't it?

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh
Member

ozh commented Nov 3, 2017

@leachim6

This comment has been minimized.

Show comment
Hide comment
@leachim6

leachim6 Nov 5, 2017

This fixed the issue for me running nginx 1.13.6-1~stretch and php 7.0+49

leachim6 commented Nov 5, 2017

This fixed the issue for me running nginx 1.13.6-1~stretch and php 7.0+49

@andrewperry

This comment has been minimized.

Show comment
Hide comment
@andrewperry

andrewperry Nov 6, 2017

I just had to install this patch to get this repo working on Debian 8

andrewperry commented Nov 6, 2017

I just had to install this patch to get this repo working on Debian 8

@LeoColomb

Tested on my side, seems much better, yep!

@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Nov 6, 2017

Member

Yeah apparently realpath() isn't the most cross platform function...

Member

ozh commented Nov 6, 2017

Yeah apparently realpath() isn't the most cross platform function...

@ozh ozh merged commit 5725e56 into master Nov 6, 2017

4 checks passed

Scrutinizer No new issues
Details
codacy/pr Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@ozh

This comment has been minimized.

Show comment
Hide comment
@ozh

ozh Nov 6, 2017

Member

(for the record and a future quick refactor: it would be more logical if YOURLS\Config\Config's constructor had two params, say $root and $config, with $root defining what we're guessing the line after with that dirname() avalanche. Would be more logical to pass that param to the class instead of practically hardcoding it within the class)

Member

ozh commented Nov 6, 2017

(for the record and a future quick refactor: it would be more logical if YOURLS\Config\Config's constructor had two params, say $root and $config, with $root defining what we're guessing the line after with that dirname() avalanche. Would be more logical to pass that param to the class instead of practically hardcoding it within the class)

@LeoColomb LeoColomb deleted the findconfig branch Nov 28, 2017

ozh referenced this pull request Dec 2, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment