Thanks for this comments > Thanks > >> 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.
In this case, we also need a specific package for the SmallLint Browser. >> - All tests should be in the Manifest-Tests package (currently, there are >> extension tests in Manifest-Core - I actually do not know why) I will check this. >> - 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. ManifestManifestCore is the manifest class of the package Manifest-Core, it does not have the subclasses. >> - 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. > > Because we do not want to rely on class refactoring. > >> - 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? > > I do not get it? It is normal code no? > You mean symbols instead of strings. > > We will the issues. > > >> >> 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." >> >> > >
