On 27 Jul 2015, at 18:11, Peter Uhn��k <i.uhnak@gmail.com> wrote:

Would be nice to have an advanced data collection, but I am not sure that I will have time now to program all that

There is already data collection for spotter and there was chatter about making the collection tool more generic, but I don't think any progress is on that.

So what exactly is "Useless" for? If there was some input box where I could describe why I think its useless in that particular context or whatever���

Yes, there should be an input box, it will be aded there as soon as I���m done with other issues and the button remains.


However I have question about two more cases, not sure if even possible to implement

case 1:
~~~~~~~~~~~~~~~~~~~~~~~~
beFinal
self isFinal: true
~~~~~~~~~~~~~~~~~~~~~~~~
"Unary "accessing" method without explicit return"

#beSomething is quite common pattern (even recommended in Kent's Best Practice Patterns), maybe it should check for name 'be'<Uppercase> before applying the critic?

This is a good proposal. Would be nice to hear other opinions. Maybe we can open an issue on the bug tracker so people can comment.



case 2:
in Roassal it's very common to create a shape but return an element
~~~~~~~~~~~~~~~~~~~~~~~~
RTEllipse new
size: 14;
color: Color black;
element
~~~~~~~~~~~~~~~~~~~~~~~~
or
~~~~~~~~~~~~~~~~~~~~~~~~
RTBox new
elementOn: aModel
~~~~~~~~~~~~~~~~~~~~~~~~

but of course it complains about missing ";yourself".

However I do not really want to ban the critic, because there may other legitimate applications in the method/class; and in Roassal-heavy code it's a nuisance.
So it would require context-aware checks (probably user-defined?) like "if receiver is subclass of RTShape then don't apply this (yourself) rule���.

Yes, it���s a very good point. It is in my plans to see how we can allow frameworks to provide their own critics and interact/override the default ones. For now all I can suggest is to ban the critic on the level of the class, or if we will reach the common sense there can be a ban on the level of package tag, so you can keep all the roassal-related classes in one tag and ban the rule for them.

Uko


Cheers,
Peter