[squeak-dev] The Trunk: Kernel-nice.1386.mcz

Nicolas Cellier nicolas.cellier.aka.nice at gmail.com
Mon Apr 19 14:04:28 UTC 2021


Hi Christoph,

Le lun. 19 avr. 2021 à 13:32, Thiede, Christoph
<Christoph.Thiede at student.hpi.uni-potsdam.de> a écrit :
>
> Hi Nicolas,
>
>
> in general, I use to submit one inbox version per proposal/fix/change. They all branch from the main Trunk line so I consider each of them something that you would call a Pull Request in the GitHub world. This also means that no inbox proposal is to be considered deprecated only because I upload a newer version. When I indeed intend to replace the former version, I usually add a notice "replaces version Package-ct.xxx" to the message of the new version. :-)
>
> I think our special problem in this case was that it is not a straightforward task to merge multiple changes of a single method with our tooling.
>

Yes, changing several times the same method is necessarily a conflict
for our tools.
IMO it's a good thing, because it will require human attention for resolution.
Merging based on line editions is not the right solution IMO.
If both editions modify the temps, they will conflict anyway.

Even if methods are atomic pieces, two modifications of the same
method are not necessarily exclusive.
It happens that we updated a comment in one branch, or renamed a
message, inst var or class in the other, etc...
But here, I failed to identify this situation.
I might have been biased by the desire to see the warnings go ;)

>
> I see the problem with annoying warnings, which is why I proposed to create a new subclass for these warnings! Then we could easily ignore these warnings when they are raised inside of the debugger. For other use cases such as automated tests or SimulationStudio, however, we would still see them.
>

Yes, this could work.
I also wondered if we could disable warnings at specific send sites on
user request (instead of disabling all the Warnings).
A good way to exercise new super powers for the Sorcerer's Apprentices ;)

>
> > I saw later that you removed <primitive: 19> from newProcess... SHould we?
>
>
> I think we should. Either a simulator client is interested to be informed about attempts of the code to escape from simulation, or it isn't. In every case, creating a process is an absolutely boring event with regard to that interest, but all actions such as resuming, terminating, or signaling a process could be of interest.
>
> > Please review the current state of Context >> #doPrimitive:method:receiver:args: and upload the missing lines again into the inbox if necessary.
>
> I'll put that onto my list, but I've myself lost track of the changes regarding primitive 186 et al. I'm not the author of these changes ...
>
> Best,
> Christoph
>
> ________________________________
> Von: Squeak-dev <squeak-dev-bounces at lists.squeakfoundation.org> im Auftrag von Nicolas Cellier <nicolas.cellier.aka.nice at gmail.com>
> Gesendet: Freitag, 16. April 2021 19:34:20
> An: The general-purpose Squeak developers list
> Betreff: Re: [squeak-dev] The Trunk: Kernel-nice.1386.mcz
>
> Hi Christoph,
> Hmm, there must have been some confusion on my side.
> I've tried to carefully review the remaining contributions in the inbox.
> To my understanding Kernel-ct.1383 fixes more simulation cases than
> Kernel-ct.1382.
> Since
> - the tests simulating primitives 83, 84 & 100 were failing,
> - and that a more recent inbox contribution was trying to fix that,
> I thought that merging this contribution was a good idea.
> It apparently was fixing two tests out of three after trial.
>
> Also, to my experience, generating a warning when we simulate
>     primitiveIndex >= 85 and: [primitiveIndex <= 88] "control primitives"])
> is super annoying! It always happens when running some kernel tests
> and forces me to switch warning off altogether.
> But switching warning off is not a good solution (and it makes some
> more tests fail !).
> Kernel-ct.1383 being more recent, I thought that the removal of
> warnings was intentional...
>
> I saw later that you removed <primitive: 19> from newProcess... SHould we?
>
> My understanding now is that you need this warning in some specific
> context. Is that it?
> We can't fix one usage by breaking another. We have to find a solution
> that fits both purposes.
>
> Le ven. 16 avr. 2021 à 15:40, Marcel Taeumel <marcel.taeumel at hpi.de> a écrit :
> >
> > Hi Christoph,
> >
> > the commit message in Kernel-nice.1386 reads: "[...] Note that Kernel-ct.1383 supersedes Kernel-ct.1382 and removes the annoying warning. [...]"
> >
> > I think the problem was that both ct.1382 and ct.1382 change the same method but do not actually consider each other. With our current tools, sub-method merging is rather tricky and hence the author should take care that this does not happen. :-)
> >
> > Please review the current state of Context >> #doPrimitive:method:receiver:args: and upload the missing lines again into the inbox if necessary.
> >
> > Best,
> > Marcel
> >
> > Am 16.04.2021 15:16:18 schrieb Christoph Thiede <christoph.thiede at student.hpi.uni-potsdam.de>:
> >
> > PS: I also see that you touched the simulation of criticalSection primitives
> > (primitiveIndex = 186 ...) again. Again, was this intended or an incident?
> > :-)
> >
> > (OT: We are really treading on each other's toes in this method ... What can
> > we learn from this? Is it too large and should be split up? Or should we
> > improve our tooling support for resolving merge conflicts?)
> >
> > Best,
> > Christoph
> >
> >
> >
> > -----
> > Carpe Squeak!
> > --
> > Sent from: http://forum.world.st/Squeak-Dev-f45488.html
> >
> >
>
>


More information about the Squeak-dev mailing list