mamuz / PhpDependencyAnalysis

Static code analysis to find violations in a dependency graph
http://mamuz.github.io/PhpDependencyAnalysis/
MIT License
561 stars 45 forks source link

Allow to use relative path in the config yaml #13

Closed fonsecas72 closed 8 years ago

fonsecas72 commented 8 years ago

Hi, I have placed a yml in one of my projects root and I have set a relative link for the "source" and "target" configurations. However, I did not started phpda from the root of my project and I got error because of it.

I think that phpda should read relative path from the location of the config yml file. I don't know if my proposal is good enough as I'm still looking to get familiar with phpda (I don't like it that much) but I wanted to ear from you before going any further.

I hope you can help me here. :)

fonsecas72 commented 8 years ago

@mamuz is this good enough?

mamuz commented 8 years ago

@fonsecas72: First of all thanks for your contribution. As you mentioned we should also support windows filesystems. Beside of this we should also think about configurations which comes directly from command line (instead of yml config file).

The symfony function "isAbsolutePath" is only working on unix systems. I prefer a solution such as PhpUnit has: https://github.com/sebastianbergmann/phpunit/blob/master/src/Util/Configuration.php#L1068

The naming of "isAbsolutePath" is in the wrong context, Context is here "AnalyzeCommand" and "AnalyzeCommand is absolute Path" is the wrong term. Something like "AnalyzeCommand generates absolute path from (path)" as a private function fits more.

Generating absolute paths should not affected arguments from command line. Therefore this behaviour should be applied before merging "config by file" and "config by cli": https://github.com/mamuz/PhpDependencyAnalysis/blob/master/src/Command/Analyze.php#L140)

fonsecas72 commented 8 years ago

Thank you @mamuz, awesome help. I totally agree with you. Let me try to adapt the PHPUnit method to this.

fonsecas72 commented 8 years ago

@mamuz Am I getting closer now? :)

mamuz commented 8 years ago

:+1: