[Pharo-dev] Pharo Quality: raising the quality level in Pharo 7 and onwards

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Fri Nov 17 08:48:21 EST 2017


It seems to me that you could technically enforce with github/travis:
- save the result of previous code critics
- pass the code critics on travis
- diff with previous
- reject any contribution creating new critics
I don't know how difficult it is to implement, nor how difficult it would
be to automatically report why the contribution was rejected (tell which
phase and provide a link to the console output...)

2017-11-17 13:03 GMT+01:00 phil at highoctane.be <phil at highoctane.be>:

> Ok. Now, how is one using those super tools?
> You mean Quality Assistant for example?
>
> Phil
>
> On Fri, Nov 17, 2017 at 11:52 AM, Torsten Bergmann <astares at gmx.de> wrote:
>
>> Hi,
>>
>> when we forked Pharo back in the days (2008) one of the primary goals of
>> Pharo was and still is to provide a CLEAN SYSTEM.
>>
>> Now after nearly 10 years we have done already an amazing job. But we are
>> still not there.
>>
>> While it is a good thing that we rewrite parts of the image, work on
>> bootstrap and new shiny tools I unfortunately see the
>> effect that even some newly written code has flaws or is not meeting some
>> basic quality standards like:
>>
>>  - having class comments
>>  - no uncategorized methods anymore
>>  - not having unused temps or unused instance variables
>>  - tests in a separate package than the code (to be able to have minified
>> versions without tests)
>>  - class instance variable naming in lowercase
>>  - ...
>>
>> Meanwhile in Pharo 6 and 7 there are nice tools inside to check your
>> code. So USE THEM.
>>
>> Some of you have already seen that I continue to do more and more of such
>> cleanups in the system in various Pharo 7
>> contributions. Believe me: this is a boring task and one can earn much
>> more applause for other nice and more amazing
>> contributions - but the dirty job needs to be done to clean up the house.
>>
>> I also started to integrate some more "Release Tests" to make sure the
>> things cleaned now stay clean over time and
>> is not violated again by new Pull requests or new package inclusions.
>>
>> Because some of these cleanups were already done in Pharo 5 and 6 - but
>> violated again by new contributions we now need
>> to enforce that the image stays at that qualit level. Because I do not
>> want to waste my time to do it over and over again!
>>
>> See package "ReleaseTests" or code like
>>
>>  ProperMethodCategorizationTest, ProperlyImplementedClassesTest,
>> #testAllClassInstanceVariablesStartLowercase and other
>>
>> which was written to ensure we not only GET better but also STAY better
>> in the future. Thanks to Pavel, Marcus, Esteban
>> and others for reviewing and integrating.
>>
>> Still there is a long journey in front of us, some examples:
>>
>>    1. Unused Temps
>>
>>          CompiledMethod allInstances select: [:m | m ast temporaries
>> anySatisfy: [:x | x binding isUnused ]].  => over 200 still
>>
>>    2. Uncategorized methods
>>
>>          Object allSubclasses select: [:each | (each selectorsInProtocol:
>> Protocol unclassified) notEmpty ]    => over 200 still
>>
>>    3. Many many more uncommented classes. We need a special reminder to
>> people who can write many code lines a day but
>>       are not able to spend a minute to write a class comment.
>>
>>    4. ...
>>
>> One person will require too much time ... several contributors helping
>> with one contribution a day or a week will already change
>> the game and make it possible again. Why not have a clean base image
>> meeting basic criterias by the end of the year 2017 and
>> ensuring with tests that we continue clean in 2018?
>>
>> Would'nt this be a nice birthday present to Pharos 10 years anniversary
>> in 2018?
>>
>> How can you help:
>> =================
>>
>>   a) you can check you own packages using the provided quality tools and
>> before you integrate new features just cleanup your code
>>      first
>>
>>   b) even as beginners you can help cleaning up such easy things and with
>> simple cleanup pull requests help moving Pharo 7 forward
>>
>>   c) you can think of other short snippets that reveal design or code
>> flaws and post them here
>>      Then we can fix these in the image too and have the rules  enforced
>> using a separate release test
>>
>>   d) lets discuss and define quality criterias and note them somewhere
>>
>>   e) ideally you can reveal problems, open bugs, clean them up and then
>> write Release tests to ensure we stay clean
>>
>>
>> We should and need to definitely raise the bar. I would even be more
>> radical to enforce more quality:
>>
>>  - in the worst case we should not accept contributions or put new
>> versions of packages into the image
>>    when they do not meet basic criterias, that is the idea of the release
>> tests
>>
>>  - if parts of the community are not seeing this as a necessity then in
>> the worst case we need to fork Pharo again
>>    into another next level CLEANED UP system ;)
>>
>>
>> So PLEASE HELP!!! Otherwise this will KILL US and it also will be and
>> ENDLESS journey for a few people to try to clean the house.
>> So PLEASE HELP before you work on the next shiny feature that might still
>> violates the basics making this an impossible journey for others
>> to move forward on that frontier.
>>
>> Bye
>> T.
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.pharo.org/pipermail/pharo-dev_lists.pharo.org/attachments/20171117/98fdc18b/attachment-0002.html>


More information about the Pharo-dev mailing list