<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html;charset=ISO-8859-1" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
In this particular case, I'd simply delete the ifFalse:. The other
changes are independent of the change by djm; they could have been made
even if version 2) never existed. The argument for this can be
strengthened because in this case it is clear why every change has been
made:<br>
3) to accomodate addition of AbstractString to the class
hierarchy <br>
4) because #isString looks better and/or is faster <br>
5) because underscores are no longer allowed for assignment)<br>
... and none of these motivations is in any way related to the change
by djm.<br>
<br>
Then, I'd advertise for volunteers to re-write the method so that it
works
when the receiver contains arbitrary objects, not only when it contains
Strings (SequenceableCollections?). A TestCase would help ensure that
they get it right.<br>
<br>
If this isn't defensible, then I don't know where we would draw the
line. <br>
<br>
Of course, there are probably many examples that are more "grey" than
this one.<br>
<br>
Anyway, thanks for highlighting these issues to the community. Cheers!<br>
Josh<br>
<br>
<br>
<br>
Ken Causey wrote:
<blockquote
cite="mid:1234121198.3474.98.camel@12-222-173-196.client.insightbb.com"
type="cite">
<pre wrap="">This is going to be a rather long email but I ask you to at least skim
if you have even a little interest in the relicensing/Squeak 4.0 effort.
Thanks to the efforts of many including Yoshiki Ohshima and Matthew
Fulmer we have made some real progress on a 'license clean' version of
Squeak. But we are now at the phase where we have some code for which
it appears we are not going to get a relicensing agreement. So at this
point it's either remove it or replace it.
Thanks to Matthew publishing the history tools I've started to try to
help out with this and have quickly learned that I really don't know
what to do. I'm going to present an example here and I'm asking you for
your considered opinion.
SequenceableCollection>>asStringWithCr
This method goes back to Squeak1.0 and has 5 versions leading up to what
is today in Squeak3.10.2-7197/Squeak4.0alpha. It is the second version
by Douglas McPherson that has the problem (we have not received a
relicensing agreement from Douglas). I'm going to paste in, oldest to
newest, all versions so we can discuss this concretely. In each I will
number the version in front of the containing filename for discussion
convenience and mark changed lines (relative to the earlier version)
with a + in the first column. The filenames used are those as
distributed with the Matthew/Yoshiki history tools; sometimes these
don't directly map to the original updates (think updates that load
MCZs).
1) SqueakV1.sources:
asStringWithCr
"Convert to a string with returns between items. Elements are usually
strings.
Useful for labels for PopUpMenus."
| labelStream |
labelStream _ WriteStream on: (String new: 200).
self do: [:each |
(each isKindOf: String)
ifTrue: [labelStream nextPutAll: each; cr]
ifFalse: [each printOn: labelStream; cr]].
self size > 0 ifTrue: [labelStream skip: -1].
^ labelStream contents!
2) 442TweaksForBeta-di.cs:
!SequenceableCollection methodsFor: 'converting' stamp: 'djm 11/20/1998
05:44'!
asStringWithCr
"Convert to a string with returns between items. Elements are
usually strings.
Useful for labels for PopUpMenus."
| labelStream |
labelStream _ WriteStream on: (String new: 200).
self do: [:each |
(each isKindOf: String)
ifTrue: [labelStream nextPutAll: each; cr]
+ ifFalse: [each printOn: labelStream. labelStream cr]].
self size > 0 ifTrue: [labelStream skip: -1].
^ labelStream contents! !
3) 5994-004-systemMod.cs:
!SequenceableCollection methodsFor: 'converting' stamp: 'yo 8/28/2002
15:39'!
asStringWithCr
"Convert to a string with returns between items. Elements are
usually strings.
Useful for labels for PopUpMenus."
| labelStream |
labelStream _ WriteStream on: (String new: 200).
self do: [:each |
+ (each isKindOf: AbstractString)
ifTrue: [labelStream nextPutAll: each; cr]
ifFalse: [each printOn: labelStream. labelStream cr]].
self size > 0 ifTrue: [labelStream skip: -1].
^ labelStream contents! !
4) 6651ExternalCleanup.cs:
!SequenceableCollection methodsFor: 'converting' stamp: 'ar 4/10/2005
18:02'!
asStringWithCr
"Convert to a string with returns between items. Elements are
usually strings.
Useful for labels for PopUpMenus."
| labelStream |
labelStream _ WriteStream on: (String new: 200).
self do: [:each |
+ each isString
ifTrue: [labelStream nextPutAll: each; cr]
ifFalse: [each printOn: labelStream. labelStream cr]].
self size > 0 ifTrue: [labelStream skip: -1].
^ labelStream contents! !
5) Squeak39g-7056+3102-7179.changes:
!SequenceableCollection methodsFor: 'converting' stamp: 'ar 4/10/2005
18:02' prior: 29748463!
asStringWithCr
"Convert to a string with returns between items. Elements are
usually strings.
Useful for labels for PopUpMenus."
| labelStream |
+ labelStream := WriteStream on: (String new: 200).
self do: [:each |
each isString
ifTrue: [labelStream nextPutAll: each; cr]
ifFalse: [each printOn: labelStream. labelStream cr]].
self size > 0 ifTrue: [labelStream skip: -1].
^ labelStream contents! !
Now as I said above the problem with with version 2. As you can see
each of these changes is at most one line and by and large frankly
rather obvious. The change in 2 is clearly a bug fix and the
straightforward solution.
So the question is what do I do with this method, which is currently
distributed as version 5, so that it does not infringe on djm's rights
to maintain version 2 as SqueakL? Admittedly it's unlikely that djm
actually wants this right, but lacking a clear statement to the
contrary, it's the default we have to assume.
How much of a change is a significant change?
Should I delete the current implementation entirely and rewrite it from
scratch? If so, how much am I allowed to know about the prior
implementations?
Ken
</pre>
<pre wrap="">
<hr size="4" width="90%">
</pre>
</blockquote>
<br>
</body>
</html>