Hi, Thanks.
I gave it a short try, and here are some comments: - The Manifest-Core should contain only the core of the manifest engine. For example, I see here some mechanism for traversing Manifest classes and retrieving data from them. Everything else that is specific to SmallLint should be in a Manifest-SmallLint package. - All tests should be in the Manifest-Tests package (currently, there are extension tests in Manifest-Core - I actually do not know why) - I like it that a Manifest class does not have to inherit from a specific root class. - There exists a ManifestManifestCore class, but I do not understand why there are subclasses to it. - You are using numbers to identify SmallLint rules. For this, you extended the rules with unique numbers. This is a very brittle scheme, and when people have domain specific rules, you will likely get a mess. Furthermore, I do not see the reason for having a number as a unique identifier, when you already have a unique identifier in the name of the rule class. Also, when looking at a method, we see something like: rule17V1FalsePositive. This is unnecessary cryptic. - In the rule methods, you are storing strings. This means that when someone renames a rule, the code in there will not be updated. Why not store this information as normal code and use symbols that can be queried? Cheers, Doru On 6 Apr 2012, at 09:51, Simon Allier wrote: > Hi pharoers > > Since a couple of months the Pharo team has been working on improving the > support for rules checking and in particular the handling of false positives > (there is nothing more boring that to get all the time the same warnings that > we know are not adequate). > We added a manifest mechanism to manage the false/true positive of SmallLint, > the rule checker of Pharo. A manifest is a class that contains meta data. In > the future we may add package license and package documentation. This data is > stored in methods on the meta side of the class. So, manifest can be > versioned in Monticello and not lost. There is one manifest per package. > > It possible to use SmallLint with Manifest using the Critics Browser > (menuWorld > Tool > CriticBrowser). With the Critics browser you can inspect > violations of rules and mark them as false positive or true positive (ToDo) > and see previous warnings flagged as false positives or toDo. > In addition all the logic of the browser has been developed so that the > manifest can be saved within the package without adding dependencies. > We will continue to improve the Browser. In addition we will run SmallLint > rules to all the Pharo packages and the rules checking will be part of the > package validation. We want the community to use rule checking for real so > that we all improve. > > Simon and Stef > > Gofer > new > url: 'http://www.squeaksource.com/PharoTaskForces'; > package: 'ConfigurationOfManifest'; > load. > ((Smalltalk at: #ConfigurationOfManifest) project version: #stable) load -- www.tudorgirba.com "Not knowing how to do something is not an argument for how it cannot be done."
