2017-09-11 12:07 GMT+02:00 Guillermo Polito <[email protected]>:
> > > On Sun, Sep 10, 2017 at 7:16 PM, Torsten Bergmann <[email protected]> wrote: > >> Hi, >> >> to explain: >> >> we have lots of uncategorized methods and non commented classes. For some >> time we had to accept this ugly situation that old and legacy code. For >> Pharo 4 and 5 >> I invested a lot of time to clean this up right before the release date. >> But with 6.0 such a round was not done and in 6.0 and 6.1 it looks like a >> mess again. >> >> Why? Because we introduced new features and code and never defined a >> certain level of quality for code we include. But our initial goal with >> Pharo was (and >> hopefully still is) to cleanup things up and do better than before. >> >> So we should not forget about quality and so I spend some time last on >> this again - now for Pharo 7. For instance #setUp and #tearDown in SUnit >> should be >> in "running" as it was defined for TestCase subclasses. Also #hash and #= >> should be in "comparing" protocol. Also the goal is coming back to an image >> where >> all methods are categorized and where all classes have a comment. >> >> But for me it absolutely makes no sense to reinvest the time over and >> over into the same cleanups while others can easily make a mess again. >> >> So we should RAISE THE QUALITY BAR to ensure that we keep with such a >> quality level. Especially as see each build to be a release and the real >> release once >> a year often is more or less a snapshot. >> >> So I also wrote a new test called "ProperMethodCategorizationTest" and >> while cleaning up #hash it was green. So ProperMethodCategorizationTest >> .testHashMethodNeedsToBeInComparingProtocol >> showed no problem and I committed and have sent a Pull request. This >> looked like any other new contribution. >> > > Yes yes, I can't agree more. We need to add Lint rules running by default > also. > I will try to prepare a small tool that will collect information about health status of a build that will be stored as a STON file next to the image (at least for the development versions). Then we can generate the same information about incoming PRs and display changes. This way we will be able to see effects of the PR like what new dependencies it added, what new rules are failing and so on. It should not reject then automatically but to be a support for the PR review. -- Pavel > > >> >> What I did not knew at this time was that Iceberg code is not in git >> repository - but managed externally. So while I fixed the code in my image >> with wrongly categorized hash >> method in an Iceberg class (IceSemanticVersion) the system did not show >> me that this change did not move to GitHub. > > >> Now I know - but this has such bad side effect when we do changes and >> these packages are in the image - but not managed with the pharo repository. >> > > Yep, this should be fixed as soon as we have multiple > subdirectories/subprojects in iceberg. As soon as that is supported we will > probably move iceberg to a subtree structure as explained in here: > > https://www.slideshare.net/GuillePolito1/understanding- > the-pharo-dev-process-79632181/1 > https://github.com/guillep/PharoIntegrationProcess/wiki/ > Analysis-of-sub-project-storage-alternatives > > > >> >> Long story short: one can not fix this situation with a simple PR as >> Iceberg code is not under the "pharo" repo umbrella. >> >> I already submitted a PR for Iceberg >> >> https://github.com/pharo-vcs/iceberg/pull/458 >> >> and Esteban included that already - but only in Iceberg. We now need to >> include Iceberg. >> >> Several lessons learned from my side: >> - doing changes and having green tests in the image is not enough >> - also doing a commit and PR does not mean all of your changes are on >> GitHub >> - it is not a good situation when a part of the image code is managed >> with "pharo" repository and another part is managed externally - I see this >> as a problem of >> the new process we should discuss and address >> - we should nonetheless try to define tests to show the edges where we >> can improve and cleanup (without a test it would not have been noticed that >> while I did my best >> to cleanup the current way of managing Iceberg prevented my fix to be >> into the image in the first place) >> > > While the test is an improvement over the current situation, I ask myself > if this particular validation of the protocols shouldn't be a Renraku rule. > > >> >> I dont know what need to be done to include a new iceberg version or when >> this will happen from Estebans side - but as soon as it is done this test >> should be green again. >> > > As Pavel said, the iceberg version that is included in the system is the > one described in > > BaselineOfIDE >> loadIceberg > > Metacello new > baseline: 'Iceberg'; > repository: 'github://pharo-vcs/iceberg:v0.5.7'; > load. > (Smalltalk classNamed: #Iceberg) enableMetacelloIntegration: true. > > So updating that method with the new iceberg version and pushing that to > Pharo should be enough. > > In any case, my mail was there just to raise the issue of the failing > tests. We should care about failing tests, and I'll cry out loud everytime > they are broken, just to make it explicit. > > >> Thanks >> T. >> >> >> Gesendet: Sonntag, 10. September 2017 um 18:24 Uhr >> Von: "Stephane Ducasse" <[email protected]> >> An: "Pharo Development List" <[email protected]> >> Betreff: Re: [Pharo-dev] argh, tests are failing! >> >> Yes we know but have no idea how to recategorise Iceberg protocols >> >> On Sun, Sep 10, 2017 at 1:03 PM, Pavel Krivanek <[email protected] >> [mailto:[email protected]]> wrote: >> >> Of course you are right, the Integrators should really take care on it. >> >> Before it were only common random failures but the main problem is with >> this PR: >> https://github.com/pharo-project/pharo/pull/264[https:// >> github.com/pharo-project/pharo/pull/264] >> >> The new test testHashMethodNeedsToBeInComparingProtocol fails on >> classIceSemanticVersion >> >> -- Pavel >> >> >> 2017-09-10 10:13 GMT+02:00 Guillermo Polito <[email protected] >> [mailto:[email protected]]>: >> Hi all, >> >> Since a couple of builds we have consistently failing the following tests: >> >> >> GT.EventRecorder.Tests.Core.GTEventRecorderTest.testDeliverNow2[ >> https://ci.inria.fr/pharo-ci-jenkins2/job/Test% >> 20pending%20pull%20request%20and%20branch%20Pipeline/job/ >> development/110/testReport/junit/GT.EventRecorder.Tests. >> Core/GTEventRecorderTest/testDeliverNow2/]GT.EventRecorder. >> Tests.Core.GTEventRecorderTest.testNotDeliveredDataShouldBeR >> esent[https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pendi >> ng%20pull%20request%20and%20branch%20Pipeline/job/develo >> pment/110/testReport/junit/GT.EventRecorder.Tests.Core/ >> GTEventRecorderTest/testNotDeliveredDataShouldBeResent/] >> Kernel.Tests.Processes.MutexTest.testFailedCriticalSectionSh >> ouldUnblockWaitingOne[https://ci.inria.fr/pharo-ci-jenkins2/ >> job/Test%20pending%20pull%20request%20and%20branch%20Pipelin >> e/job/development/110/testReport/junit/Kernel.Tests. >> Processes/MutexTest/testFailedCriticalSectionShouldUnblockWa >> itingOne/]ReleaseTests.Categorization.ProperMethodCat >> egorizationTest.testHashMethodNeedsToBeInComparingProtocol[ >> https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending% >> 20pull%20request%20and%20branch%20Pipeline/job/develo >> pment/110/testReport/junit/ReleaseTests.Categorization/Pr >> operMethodCategorizationTest/testHashMethodNeedsToBeInCompar >> ingProtocol/]ReleaseTests.Categorization.ProperMethodCategor >> izationTest.testHashMethodNeedsToBeInComparingProtocol[ >> https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending% >> 20pull%20request%20and%20branch%20Pipeline/job/develo >> pment/110/testReport/junit/ReleaseTests.Categorization/Pr >> operMethodCategorizationTest/testHashMethodNeedsToBeInCompar >> ingProtocol_2/]ReleaseTests.Categorization.ProperMethodCateg >> orizationTest.testHashMethodNeedsToBeInComparingProtocol[ >> https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending% >> 20pull%20request%20and%20branch%20Pipeline/job/develo >> pment/110/testReport/junit/ReleaseTests.Categorization/Pr >> operMethodCategorizationTest/testHashMethodNeedsToBeInCompar >> ingProtocol_3/]ReleaseTests.Categorization.ProperMethodCateg >> orizationTest.testHashMethodNeedsToBeInComparingProtocol[ >> https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending% >> 20pull%20request%20and%20branch%20Pipeline/job/develo >> pment/110/testReport/junit/ReleaseTests.Categorization/Pr >> operMethodCategorizationTest/testHashMethodNeedsToBeInCompar >> ingProtocol_4/]ReleaseTests.Categorization.ProperMethodCateg >> orizationTest.testHashMethodNeedsToBeInComparingProtocol[ >> https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending% >> 20pull%20request%20and%20branch%20Pipeline/job/develo >> pment/110/testReport/junit/ReleaseTests.Categorization/Pr >> operMethodCategorizationTest/testHashMethodNeedsToBeInCompar >> ingProtocol_5/]ReleaseTests.Categorization.ProperMethodCateg >> orizationTest.testHashMethodNeedsToBeInComparingProtocol[ >> https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending% >> 20pull%20request%20and%20branch%20Pipeline/job/develo >> pment/110/testReport/junit/ReleaseTests.Categorization/Pr >> operMethodCategorizationTest/testHashMethodNeedsToBeInCompar >> ingProtocol_6/]ReleaseTests.Categorization.ProperMethodCateg >> orizationTest.testHashMethodNeedsToBeInComparingProtocol[ >> https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending% >> 20pull%20request%20and%20branch%20Pipeline/job/develo >> pment/110/testReport/junit/ReleaseTests.Categorization/Pr >> operMethodCategorizationTest/testHashMethodNeedsToBeInCompar >> ingProtocol_7/ >> <https://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/GT.EventRecorder.Tests.Core/GTEventRecorderTest/testDeliverNow2/%5DGT.EventRecorder.Tests.Core.GTEventRecorderTest.testNotDeliveredDataShouldBeResent%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/GT.EventRecorder.Tests.Core/GTEventRecorderTest/testNotDeliveredDataShouldBeResent/%5DKernel.Tests.Processes.MutexTest.testFailedCriticalSectionShouldUnblockWaitingOne%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/Kernel.Tests.Processes/MutexTest/testFailedCriticalSectionShouldUnblockWaitingOne/%5DReleaseTests.Categorization.ProperMethodCategorizationTest.testHashMethodNeedsToBeInComparingProtocol%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/testHashMethodNeedsToBeInComparingProtocol/%5DReleaseTests.Categorization.ProperMethodCategorizationTest.testHashMethodNeedsToBeInComparingProtocol%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/testHashMethodNeedsToBeInComparingProtocol_2/%5DReleaseTests.Categorization.ProperMethodCategorizationTest.testHashMethodNeedsToBeInComparingProtocol%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/testHashMethodNeedsToBeInComparingProtocol_3/%5DReleaseTests.Categorization.ProperMethodCategorizationTest.testHashMethodNeedsToBeInComparingProtocol%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/testHashMethodNeedsToBeInComparingProtocol_4/%5DReleaseTests.Categorization.ProperMethodCategorizationTest.testHashMethodNeedsToBeInComparingProtocol%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/testHashMethodNeedsToBeInComparingProtocol_5/%5DReleaseTests.Categorization.ProperMethodCategorizationTest.testHashMethodNeedsToBeInComparingProtocol%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/testHashMethodNeedsToBeInComparingProtocol_6/%5DReleaseTests.Categorization.ProperMethodCategorizationTest.testHashMethodNeedsToBeInComparingProtocol%5Bhttps://ci.inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request%20and%20branch%20Pipeline/job/development/110/testReport/junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/testHashMethodNeedsToBeInComparingProtocol_7/> >> ] >> >> >> Green builds are the only hard metric to say if the build is healthy or >> not. >> >> - We should not integrate anything until the build is green again... >> >> Also, we spent with Pablo a lot of time to have a green build in all >> platforms... I'd like to spend my time in other fun stuff than the CI :/ >> >> -- >> >> >> Guille Polito >> >> Research Engineer >> French National Center for Scientific Research - http://www.cnrs.fr[ >> http://www.cnrs.fr] >> >> >> Web: http://guillep.github.io[http://guillep.github.io] >> Phone: +33 06 52 70 66 13 >> >> > > > -- > > > > Guille Polito > > > Research Engineer > > French National Center for Scientific Research - *http://www.cnrs.fr* > <http://www.cnrs.fr> > > > > *Web:* *http://guillep.github.io* <http://guillep.github.io> > > *Phone: *+33 06 52 70 66 13 <+33%206%2052%2070%2066%2013> >
