Hi, On Tue, Apr 10, 2012 at 2:18 PM, Simon Allier <[email protected]> wrote: > 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.
Indeed. It was my bad. Doru >>> - 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." >>> >>> >> >> > > -- www.tudorgirba.com "Every thing has its own flow"
