-
Notifications
You must be signed in to change notification settings - Fork 6
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
Define "legacy exclusions" as discouraged rules #7
Comments
I'm trying to figure out if this approach is useable in our context. We want to use this tool on our CI to forbid new violations, in this context, it's only useable as "cool not strict mode" because we want 0 issues and raise only new ones. Problem is if a Rule is "discouraged", until we fix it and pass it as "forbidden", new issues can occur with this not enforced rule. Today, with our 7 rules, 4 rules cannot be used as "forbidden":
Let's take this last issue as example, the Rule "Pim\Bundle\ConnectorBundle" violated in file "src/Pim/Bundle/ConnectorBundle/Writer/File/ContextableCsvWriter.php" because use Pim\Bundle\BaseConnectorBundle\Writer\File\CsvWriter ContextableCsvWriter has been introduced in a 1.4 patch in a no BC mode, in 1.5 we re-worked/moved it, we depreciate this class with an expected removing in 1.6. Means the rule "Pim\Bundle\ConnectorBundle" cannot be used as "forbidden" until 1.6 and we can introduce new violations on new developments during this period because we cannot use this rule as strict. We could split these rules in smaller rules or deal with exclude inside the rules with regex but it will make the definition of rules far harder. We could also define 'only' rules but issue with this approach is we have to list all "uses", quite doable in a already defined component for instance, but far more harder in a brand new dev or in a bundle which move a lot (EnrichBundle for instance). Digging and trying stuff that i will share here :) |
Trying to compare with other tools, it's like if when we use phpmd we say the CyclomaticComplexity rule is not enforced at all because i already have 10 violations. In this case, the use of 10 "suppressWarning" option seems a good compromise to me because it's less permissive than totally muting the rule and forbid new violations to occur. In phpmd this "suppressWarning" is grabbed from the Node because they use annotations, here i would prefer a configuration not enforced in the code itself but that you can easily define in the conf of the project. cf http://phpmd.org/rules/codesize.html + http://phpmd.org/documentation/suppress-warnings.html |
@jjanvier i'm continuing to dig into our real use cases related to violations to see if most of these can be easily fixed and what are those we should keep for longer time due to deprecated strategy (for instance by moving catalog bundle models to catalog component we solve 144 / 367 violations in CE). |
in strict mode, discouraged rules are counted and displayed as violations
in "cool" mode, they are not
The text was updated successfully, but these errors were encountered: