Hi,
On Sat, Apr 7, 2012 at 9:14 AM, Stéphane Ducasse
<[email protected]> wrote:
> 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.
>> - 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.
>
> Because we do not want to rely on class refactoring.
I do not get it. You want an identifier. Why is a brittle and
obfuscated number better than the name of the class, which is already
a proper identifier? If you do not want to rely directly on a class
reference, you can store it as a symbol. If you keep the numbering,
you will immediately get into numbering issues as soon as people will
start writing their own rules.
>> - 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.
I mean you are currently storing the following in a rule exception:
rule24V1FalsePositive
^ #(#('(RGMethodDefinition className: ''RGClassDefinitionTest''
selector: #testReadFrom isMetaSide: false)'
#'2012-04-02T11:31:59.933+02:00') )
This an array of strings, not code. I was wondering why it would not
be possible to be code directly.
Cheers,
Doru
> 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"