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 [email protected] <[email protected]>: > 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 <[email protected]> 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. >> >> >
