On Mon, Sep 11, 2017 at 7:19 AM, Stephane Ducasse <[email protected]> wrote:
> Hi torsten > > From a process I think that we decided that external packages should > be managed like the following: > > - PR (from people) to change the code in Pharo > + issues a PR to Iceberg. > > - then Iceberg team merge PR + > issue a new PR for Pharo integration > > So I think that this is what you describe too? > Yes, that's what I described in ESUG too :) https://www.slideshare.net/GuillePolito1/understanding-the-pharo-dev-process-79632181/34?src=clipshare On the way back from ESUG we discussed with Esteban a way to support this kind of scenarios cleanly and without needing too much of a refactor ;) > > Like that we do not have deadlock and can always have a stable version. > > Stef > > > > > > > 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. > > > > 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. > > > > 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) > > > > 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. > > > > 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. > testNotDeliveredDataShouldBeResent[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/ > testNotDeliveredDataShouldBeResent/]Kernel.Tests.Processes.MutexTest. > testFailedCriticalSectionShouldUnblockWaitingOne[https://ci. > inria.fr/pharo-ci-jenkins2/job/Test%20pending%20pull% > 20request%20and%20branch%20Pipeline/job/development/ > 110/testReport/junit/Kernel.Tests.Processes/MutexTest/ > testFailedCriticalSectionShouldUnblockWaitingOne/] > ReleaseTests.Categorization.ProperMethodCategorizationTest. > testHashMethodNeedsToBeInComparingProtocol[https://ci.inria. > fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request% > 20and%20branch%20Pipeline/job/development/110/testReport/ > junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/ > testHashMethodNeedsToBeInComparingProtocol/]ReleaseTests.Categorization. > ProperMethodCategorizationTest.testHashMethodNeedsToBeInCompa > ringProtocol[https://ci.inria.fr/pharo-ci-jenkins2/job/Test% > 20pending%20pull%20request%20and%20branch%20Pipeline/job/ > development/110/testReport/junit/ReleaseTests.Categorization/ > ProperMethodCategorizationTest/testHashMethodNeedsToBeInCompa > ringProtocol_2/]ReleaseTests.Categorization.ProperMethodCategorizationTest > .testHashMethodNeedsToBeInComparingProtocol[https://ci.inria. > fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request% > 20and%20branch%20Pipeline/job/development/110/testReport/ > junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/ > testHashMethodNeedsToBeInComparingProtocol_3/]ReleaseTests.Categorization. > ProperMethodCategorizationTest.testHashMethodNeedsToBeInCompa > ringProtocol[https://ci.inria.fr/pharo-ci-jenkins2/job/Test% > 20pending%20pull%20request%20and%20branch%20Pipeline/job/ > development/110/testReport/junit/ReleaseTests.Categorization/ > ProperMethodCategorizationTest/testHashMethodNeedsToBeInCompa > ringProtocol_4/]ReleaseTests.Categorization.ProperMethodCategorizationTest > .testHashMethodNeedsToBeInComparingProtocol[https://ci.inria. > fr/pharo-ci-jenkins2/job/Test%20pending%20pull%20request% > 20and%20branch%20Pipeline/job/development/110/testReport/ > junit/ReleaseTests.Categorization/ProperMethodCategorizationTest/ > testHashMethodNeedsToBeInComparingProtocol_5/]ReleaseTests.Categorization. > ProperMethodCategorizationTest.testHashMethodNeedsToBeInCompa > ringProtocol[https://ci.inria.fr/pharo-ci-jenkins2/job/Test% > 20pending%20pull%20request%20and%20branch%20Pipeline/job/ > development/110/testReport/junit/ReleaseTests.Categorization/ > ProperMethodCategorizationTest/testHashMethodNeedsToBeInCompa > ringProtocol_6/]ReleaseTests.Categorization.ProperMethodCategorizationTest > .testHashMethodNeedsToBeInComparingProtocol[https://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
