Skip to content

Conversation

@frenck
Copy link
Contributor

@frenck frenck commented May 10, 2017

Proposed Changes

Changed they way paths are put into PHP_CodeSniffer based on Composer being executes globally or not. Local repositories (project folder) installations will use relative paths from the PHP_CodeSniffer installation directory to the directory containing the coding standard.

The behaviour for globally installed instances will keep using absolute paths; unchanged.

Related Issues

#20 Install sniffs with relative paths in CodeSniffer.conf

Copy link
Member

@Potherca Potherca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing weird found. Fix comments at your leisure. (Or not).

src/Plugin.php Outdated
$changes = false;
foreach ($this->installedPaths as $key => $path) {
if (file_exists($path) === false || is_dir($path) === false || is_readable($path) === false) {
// This might be an relative path as well
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo:

a relative path


$finder = new Finder();
$finder->files()
->ignoreUnreadableDirs()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!


/**
* Test if composer is running "global"
* This check kinda dirty, but it is the "Composer Way"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not Sure If...

src/Plugin.php Outdated

// Some compatibility fixes for Windows paths
$from = is_dir($from) ? rtrim($from, '\/') . '/' : $from;
$to = is_dir($to) ? rtrim($to, '\/') . '/' : $to;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too much whitespace between ) ?.

*
* @return string Relative path
*/
private function getRelativePath($to)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is going to be a bastard to test 😿

@frenck frenck force-pushed the feature/relative-paths-support branch from c9b7c6b to d39ee79 Compare May 10, 2017 12:45
@frenck frenck merged commit 2250c53 into master May 10, 2017
@frenck frenck deleted the feature/relative-paths-support branch May 10, 2017 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants