-
Notifications
You must be signed in to change notification settings - Fork 133
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
Implemented skip violations #200
Conversation
thanks for trying to make deptrac better, i am unsure about this PR. I would agree that we need some way to skip violations but there are a bunch of issues with this approach. when i wrote deptrac i had the idea that a dependency is something abstract. in fact we (mostly) use deptrac on the "class"-level but if we think further dependencies has nothing to do with classes. for example we could write a npath collector that collects every function that is "too complex". than you could write a rule to prevent some parts of your code to prevent on too complex code. You would end up with a dependency between (may) 2 functions, not classes. atm. i am unsure about how to solve this issue but i guess the approach is not generic enough. |
@timglabisch agree with you. When you integrate DepTrac in CI/CD with large code base (our case) all dependencies cannot be easily fixed, because it would take a lot of developer-hour. IMO DepTrac definitely should support violation skip possibility otherwise integration process might not be so smooth. |
@timglabisch as you can see in #199 we have implemented a workaround using existing functionality, but not the best option. We would like to see jUnit support as well to see and control the amount of existing skipped violations. Also, an approach described in #199 provides a possibility to skip some class completely from an analysis, but in the scope of this PR I have implemented a possibility to skip exact violation which minimizes probability to introduce new violations. |
@timglabisch any ideas? Should I continue work on PR or you will just close it? |
I think this would be good start having a skipping mechanism. If there is somebody having the use case skipping functions/npath then they/we should start thinking about abstracting the mechanism. @torinaki 👍 feel free to continue |
@torinaki Some documentation in the |
23cf08f
to
1c3e972
Compare
@smoench , @timglabisch it seems that PR is ready for review. |
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.
except for the little things 👍
@torinaki Would be great if you could rebase on actual master. I extended the travis ci config to enforce static analysis. As soon as the pipeline gets green I will merge your PR :) |
7a895db
to
0f73927
Compare
@torinaki It seems you have just a few coding style issues. They could be easily fixed with php-cs-fixer. Just download the latest phar-file and run following command |
0f73927
to
f7abacc
Compare
@smoench CS is fixed |
Thank you @torinaki |
Initial implementation of #199
Scope
Implement possibility to skip class dependency violation in
depfile.yml
. There will be new configuration sectionskip_violations
(maybeskipped_violations
better?). The developer can configure it in the following way:skip_violations
section contains associative array where key (Library\LibClass
) is the name of dependant class and values (Core\CoreClass
,Core\AnotherClass
) are dependency classes.Future scope
There might be implemented possibility for PHP functions dependency as well. In this case, we can specify functions names instead of class names. PHP allows to create functions with names which can be equal to class names in the same namespace:
output:
So function names should be identified differently from clasess. We can sufix each function name with
()
:TODO