Hi, On Tue, Apr 10, 2012 at 2:18 PM, Simon Allier <simon.allier@inria.fr> 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"