Seaside icon indicating copy to clipboard operation
Seaside copied to clipboard

Seaside-Pharo-REST-Core should be for Squeak and Seaside-Pharo20-REST-Core for Pharo

Open jecisc opened this issue 5 years ago • 4 comments

jecisc avatar Dec 06 '18 00:12 jecisc

Hum... Looking at it I have the impression that we could just move the content of those packages to Grease.

Those two packages contain only one method in two versions:

Seaside-Pharo-REST-Core

argumentNamesOf: aCompiledMethod
	^ aCompiledMethod methodNode arguments collect: [ :each | each key ]

Seaside-Pharo20-REST-Core

argumentNamesOf: aCompiledMethod
	^ aCompiledMethod argumentNames

@jbrichau What do you think? I would be in favor of adding those two methods in the Squeak, Pharo and Gemstone platform classes in Grease and remove the two packages to reduce complexity of Seaside.

jecisc avatar Dec 06 '18 00:12 jecisc

In case you agree, for reference, here is the Gemstone version:

argumentNamesOf: aMethod

	^(aMethod argsAndTemps copyFrom: 1 to: aMethod numArgs) collect: [:each | each asString ]

jecisc avatar Dec 06 '18 00:12 jecisc

Could be moved to Grease indeed. I think the reason not move was because it's only applicable to Seaside-REST. Then again, this complicates matters in the baseline.

I would favour moving this up to Grease yes. What do you think @marschall ?

jbrichau avatar Dec 16 '18 16:12 jbrichau

We could move it to Grease, opening again the eternal question what should be in Grease and what what not. It would help a bit with the baseline for Seaside but we would have to make sure it's in all the Pharo and Squeak versions and all the ports would have to add it. On the other hand we get rid of a package that contains only a single method.

The other alternative as @jecisc mentioned would be to rename Seaside-Pharo20-REST-Core to Seaside-Pharo-REST-Core and Seaside-Pharo-REST-Core to Seaside-Squeak-REST-Core.

I'm fine with both solutions.

marschall avatar Aug 28 '19 09:08 marschall