[Pharo-project] SmallLint and manifest
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
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."
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.
- 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."
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."
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"
Hi, On Sat, Apr 7, 2012 at 9:14 AM, Stéphane Ducasse <stephane.ducasse@inria.fr> 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"
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.
sure this is not the problem numbers solve. We want to have no impact because of class name change.
If you keep the numbering, you will immediately get into numbering issues as soon as people will start writing their own rules.
Normally not because number should be unique and should never change. But I see your point After we have a version number when we change a rule for example rule 1 version 1 is different from rule 1 version 2 How you scenario behave in presence of class renaming? Our is stable but indeed people can add number that clashes with existing ones. Now we should have a rule stating that rule should have unique id.
- 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.
because we do not want to store information that force us to have ring loaded. 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"
Hi, On 10 Apr 2012, at 21:31, Stéphane Ducasse wrote:
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.
sure this is not the problem numbers solve. We want to have no impact because of class name change.
If you keep the numbering, you will immediately get into numbering issues as soon as people will start writing their own rules.
Normally not because number should be unique and should never change. But I see your point After we have a version number when we change a rule for example rule 1 version 1 is different from rule 1 version 2
How you scenario behave in presence of class renaming? Our is stable but indeed people can add number that clashes with existing ones. Now we should have a rule stating that rule should have unique id.
Rules are effective when they are like specific, not when they are generic. So, we should expect (and encourage) people to write their own rules for their own context. There is no way in which you could get a numbering scheme to fit a hundred different projects and still have an understandable situation. In my scenario, if someone renamed a SmallLint rule but the false positives still refer to the old one, the false positives from the manifest would simply get ignored. On top of that, we could add a meta-rule that says that all manifest rules should have a corresponding SamllLint rule.
- 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.
because we do not want to store information that force us to have ring loaded.
Why not: '(RGMethodDefinition className: ''RGClassDefinitionTest'' selector: #testReadFrom isMetaSide: false)' ==> (Smalltalk at: #RGMethodDefinition) className: ''RGClassDefinitionTest'' selector: #testReadFrom isMetaSide: false Cheers, Doru
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"
-- www.tudorgirba.com "We are all great at making mistakes."
Hi, i have published a new version: - the package Manifest-Core is split in two packages: Manifest-Core and Manifest-CriticBrowser. - the unique identifier for a rule is now a string, not a number. For example, in the manifest we have ruleBadMessageRuleV1FalsePositive rather than ruleBadMessageRuleV1FalsePositive. Simon On Apr 10, 2012, at 9:57 PM, Tudor Girba wrote:
Hi,
On 10 Apr 2012, at 21:31, Stéphane Ducasse wrote:
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.
sure this is not the problem numbers solve. We want to have no impact because of class name change.
If you keep the numbering, you will immediately get into numbering issues as soon as people will start writing their own rules.
Normally not because number should be unique and should never change. But I see your point After we have a version number when we change a rule for example rule 1 version 1 is different from rule 1 version 2
How you scenario behave in presence of class renaming? Our is stable but indeed people can add number that clashes with existing ones. Now we should have a rule stating that rule should have unique id.
Rules are effective when they are like specific, not when they are generic. So, we should expect (and encourage) people to write their own rules for their own context. There is no way in which you could get a numbering scheme to fit a hundred different projects and still have an understandable situation.
In my scenario, if someone renamed a SmallLint rule but the false positives still refer to the old one, the false positives from the manifest would simply get ignored. On top of that, we could add a meta-rule that says that all manifest rules should have a corresponding SamllLint rule.
- 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.
because we do not want to store information that force us to have ring loaded.
Why not:
'(RGMethodDefinition className: ''RGClassDefinitionTest'' selector: #testReadFrom isMetaSide: false)'
==>
(Smalltalk at: #RGMethodDefinition) className: ''RGClassDefinitionTest'' selector: #testReadFrom isMetaSide: false
Cheers, Doru
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"
-- www.tudorgirba.com
"We are all great at making mistakes."
Thanks simon. I will try on citizen and Kozen then after on all the packages I can find and I know :) Gofer new url: 'http://www.squeaksource.com/PharoTaskForces'; package: 'ConfigurationOfManifest'; load. ((Smalltalk at: #ConfigurationOfManifest) project version: #stable) load On Apr 18, 2012, at 11:39 AM, Simon Allier wrote:
Hi,
i have published a new version: - the package Manifest-Core is split in two packages: Manifest-Core and Manifest-CriticBrowser. - the unique identifier for a rule is now a string, not a number. For example, in the manifest we have ruleBadMessageRuleV1FalsePositive rather than ruleBadMessageRuleV1FalsePositive.
Simon
On Apr 10, 2012, at 9:57 PM, Tudor Girba wrote:
Hi,
On 10 Apr 2012, at 21:31, Stéphane Ducasse wrote:
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.
sure this is not the problem numbers solve. We want to have no impact because of class name change.
If you keep the numbering, you will immediately get into numbering issues as soon as people will start writing their own rules.
Normally not because number should be unique and should never change. But I see your point After we have a version number when we change a rule for example rule 1 version 1 is different from rule 1 version 2
How you scenario behave in presence of class renaming? Our is stable but indeed people can add number that clashes with existing ones. Now we should have a rule stating that rule should have unique id.
Rules are effective when they are like specific, not when they are generic. So, we should expect (and encourage) people to write their own rules for their own context. There is no way in which you could get a numbering scheme to fit a hundred different projects and still have an understandable situation.
In my scenario, if someone renamed a SmallLint rule but the false positives still refer to the old one, the false positives from the manifest would simply get ignored. On top of that, we could add a meta-rule that says that all manifest rules should have a corresponding SamllLint rule.
- 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.
because we do not want to store information that force us to have ring loaded.
Why not:
'(RGMethodDefinition className: ''RGClassDefinitionTest'' selector: #testReadFrom isMetaSide: false)'
==>
(Smalltalk at: #RGMethodDefinition) className: ''RGClassDefinitionTest'' selector: #testReadFrom isMetaSide: false
Cheers, Doru
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"
-- www.tudorgirba.com
"We are all great at making mistakes."
participants (3)
-
Simon Allier -
Stéphane Ducasse -
Tudor Girba