Smalltalk › Pharo › Pharo Smalltalk Developers

Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

_‹ Previous Topic Next Topic _›

Classic

List

Threaded

13 messages Options

Torsten Bergmann

Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

Hi,

just as info: looks like already fixed Pharo issue #11876 makes problem for Seaside in 3.0:

http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005509.html
http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005540.html

Thx
T.

Stéphane Ducasse

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

> Hi,
>
> just as info: looks like already fixed Pharo issue #11876 makes problem for Seaside in 3.0:
>
> http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005509.html
> http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005540.html

It would be a bit sad that Seaside does not work in Pharo3.0
I imagine that Philippe is tired.

I would revert the change and add a rule that check that the argument of shouldnt:raise: should not be an Error

Stef

> Thx
> T.
>

Marcus Denker-4

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

On 23 Oct 2013, at 23:07, Stéphane Ducasse <[hidden email]> wrote:

>
>> Hi,
>>
>> just as info: looks like already fixed Pharo issue #11876 makes problem for Seaside in 3.0:
>>
>> http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005509.html
>> http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005540.html
>
>
> It would be a bit sad that Seaside does not work in Pharo3.0
> I imagine that Philippe is tired.
>
> I would revert the change and add a rule that check that the argument of shouldnt:raise: should not be an Error
>

yes, people are not willing to change code all the time… we should do things like that in rules first and only later
hard errors.

Marcus

Johan Brichau-2

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

On 24 Oct 2013, at 08:05, Marcus Denker <[hidden email]> wrote:

>
> On 23 Oct 2013, at 23:07, Stéphane Ducasse <[hidden email]> wrote:
>
>>
>>> Hi,
>>>
>>> just as info: looks like already fixed Pharo issue #11876 makes problem for Seaside in 3.0:
>>>
>>> http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005509.html
>>> http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005540.html
>>
>>
>> It would be a bit sad that Seaside does not work in Pharo3.0
>> I imagine that Philippe is tired.
>>
>> I would revert the change and add a rule that check that the argument of shouldnt:raise: should not be an Error
>>
>
> yes, people are not willing to change code all the time… we should do things like that in rules first and only later
> hard errors.

+ 1

Philippe Marschall-2

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

In reply to this post by Marcus Denker-4

On 24.10.13 08:05, Marcus Denker wrote:

We are willing to change the code where there is a clear benefit. For
example it's hard to argue that the new filesystem is an improvement
over FileDirectory. We have a partically working Spec based UI for the
servers. We would happily change the walkback to a new context API
should it fix the bugs in the existing one. However this isn't the case
with this change. To sole and only purpose of this change is to break
code. This is what annoys me.

Cheers
Philippe

NorbertHartl

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

In reply to this post by Marcus Denker-4

Am 24.10.2013 um 08:05 schrieb Marcus Denker <[hidden email]>:

On 23 Oct 2013, at 23:07, Stéphane Ducasse <[hidden email]> wrote:

Hi,

just as info: looks like already fixed Pharo issue #11876 makes problem for Seaside in 3.0:

http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005509.html
http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005540.html

It would be a bit sad that Seaside does not work in Pharo3.0
I imagine that Philippe is tired.

I would revert the change and add a rule that check that the argument of shouldnt:raise: should not be an Error

yes, people are not willing to change code all the time… we should do things like that in rules first and only later
hard errors.

Norbert

NorbertHartl

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

In reply to this post by Philippe Marschall-2

Am 24.10.2013 um 10:47 schrieb Philippe Marschall <[hidden email]>:

On 24.10.13 08:05, Marcus Denker wrote:

On 23 Oct 2013, at 23:07, Stéphane Ducasse <[hidden email]> wrote:

Hi,

just as info: looks like already fixed Pharo issue #11876 makes problem for Seaside in 3.0:

http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005509.html
http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005540.html

It would be a bit sad that Seaside does not work in Pharo3.0
I imagine that Philippe is tired.

I would revert the change and add a rule that check that the argument of shouldnt:raise: should not be an Error

yes, people are not willing to change code all the time…

We are willing to change the code where there is a clear benefit. For example it's hard to argue that the new filesystem is an improvement over FileDirectory. We have a partically working Spec based UI for the servers. We would happily change the walkback to a new context API should it fix the bugs in the existing one. However this isn't the case with this change. To sole and only purpose of this change is to break code. This is what annoys me.

Personally I think the new filesystem is way better than the old one. Maybe not for seaside that has rather basic usage of it. This change I would consider game changer for pharo that need to happen from time to time.

But with arbitrary changes we need to be very careful. There needs to be change because we cannot evolve without. But there is a border where changes are just for the sake of it (hard to justify). And that would turn pharo into _one of those_ open source projects where the only thing that is stable is the number of bugs.

Norbert

Torsten Bergmann

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

From the comments I think we can agree that it is a better to have a lint rule

instead of breaking things the way it was introduced by the original issue #11876.

I therefore opened an issue

https://pharo.fogbugz.com/f/cases/11989/shouldnt-raise-Error-from-issue-11876-is-too-offensive

and uploaded a slice

SLICE-Issue-11989-shouldntraise-Error-from-issue-11876-is-too-offensive-TorstenBergmann.1

to Pharo30Inbox on STHub (http://smalltalkhub.com/mc/Pharo/Pharo30Inbox/main) that

removed #validateShouldntException:

I used a fresh 3.0 image (Pharo3.0 Latest update: #30519)

Please test so it can be integrated and we can continue to focus on more interesting stuff.

Thanks

Torsten

Sven Van Caekenberghe-2

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

On 24 Oct 2013, at 13:55, Torsten Bergmann <[hidden email]> wrote:

> From the comments I think we can agree that it is a better to have a lint rule
> instead of breaking things the way it was introduced by the original issue #11876.
>
> I therefore opened an issue
> https://pharo.fogbugz.com/f/cases/11989/shouldnt-raise-Error-from-issue-11876-is-too-offensive
> and uploaded a slice
>
> SLICE-Issue-11989-shouldntraise-Error-from-issue-11876-is-too-offensive-TorstenBergmann.1
>
> to Pharo30Inbox on STHub (http://smalltalkhub.com/mc/Pharo/Pharo30Inbox/main) that
> removed #validateShouldntException:
>
> I used a fresh 3.0 image (Pharo3.0 Latest update: #30519)
>
> Please test so it can be integrated and we can continue to focus on more interesting stuff.

With respect to Seaside (and its cross-platform-ness), this is probably a good decision, for now.

But it is fundamentally unfair to call this an arbitrary unimportant change meant to annoy people. On the contrary ! I even think that there are too many methods in TAssertable and the #shouldnt: checks are very confusing and are candidates for removal, IMHO. Too much unnecessary code is not good in the long run.

Have you ever written a #shouldnt: test ? I have never done so !

I took a Seaside image and looked at the senders of #shouldnt: and friends. There are that many from Seaside and I haven’t seen any good example that could not be written otherwise.

My 2c.

Sven

> Thanks
> Torsten
>
>
>

Philippe Marschall-2

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

In reply to this post by NorbertHartl

On 24.10.13 11:59, Norbert Hartl wrote:

>
> Am 24.10.2013 um 10:47 schrieb Philippe Marschall
> <[hidden email]
> <mailto:[hidden email]>>:
>
>> On 24.10.13 08:05, Marcus Denker wrote:
>>>
>>> On 23 Oct 2013, at 23:07, Stéphane Ducasse
>>> <[hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>
>>>>
>>>>> Hi,
>>>>>
>>>>> just as info: looks like already fixed Pharo issue #11876 makes
>>>>> problem for Seaside in 3.0:
>>>>>
>>>>> http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005509.html
>>>>> http://lists.squeakfoundation.org/pipermail/seaside-dev/2013-October/005540.html
>>>>
>>>>
>>>> It would be a bit sad that Seaside does not work in Pharo3.0
>>>> I imagine that Philippe is tired.
>>>>
>>>> I would revert the change and add a rule that check that the
>>>> argument of shouldnt:raise: should not be an Error
>>>>
>>>
>>> yes, people are not willing to change code all the time…
>>
>> We are willing to change the code where there is a clear benefit. For
>> example it's hard to argue that the new filesystem is an improvement
>> over FileDirectory. We have a partically working Spec based UI for the
>> servers. We would happily change the walkback to a new context API
>> should it fix the bugs in the existing one. However this isn't the
>> case with this change. To sole and only purpose of this change is to
>> break code. This is what annoys me.
>>
> Personally I think the new filesystem is way better than the old one.

Yeah, I meant it's better. I guess I should proof read my mails more.

Cheers
Philippe

Stéphane Ducasse

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

In reply to this post by Philippe Marschall-2

>>>
>>
>> yes, people are not willing to change code all the time…
>
> We are willing to change the code where there is a clear benefit. For example it's hard to argue that the new filesystem is an improvement over FileDirectory. We have a partically working Spec based UI for the servers. We would happily change the walkback to a new context API should it fix the bugs in the existing one. However this isn't the case with this change. To sole and only purpose of this change is to break code. This is what annoys me.

don't worry we understand :)
We are building real applications too for real clients too :)
This is a one line change and we will revert it.
Stef
>
> Cheers
> Philippe
>
>
>

Stéphane Ducasse

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

In reply to this post by Torsten Bergmann

tx!

On Oct 24, 2013, at 1:55 PM, Torsten Bergmann <[hidden email]> wrote:

From the comments I think we can agree that it is a better to have a lint rule

instead of breaking things the way it was introduced by the original issue #11876.

I therefore opened an issue

https://pharo.fogbugz.com/f/cases/11989/shouldnt-raise-Error-from-issue-11876-is-too-offensive

and uploaded a slice

SLICE-Issue-11989-shouldntraise-Error-from-issue-11876-is-too-offensive-TorstenBergmann.1

to Pharo30Inbox on STHub (http://smalltalkhub.com/mc/Pharo/Pharo30Inbox/main) that

removed #validateShouldntException:

I used a fresh 3.0 image (Pharo3.0 Latest update: #30519)

Please test so it can be integrated and we can continue to focus on more interesting stuff.

Thanks

Torsten

Stéphane Ducasse

Re: Issue #11876 (Do not allow shouldnt:raise: Error) and Seaside dev. on Pharo 3.0

In reply to this post by Sven Van Caekenberghe-2

>
> With respect to Seaside (and its cross-platform-ness), this is probably a good decision, for now.
>
> But it is fundamentally unfair to call this an arbitrary unimportant change meant to annoy people. On the contrary ! I even think that there are too many methods in TAssertable and the #shouldnt: checks are very confusing and are candidates for removal, IMHO. Too much unnecessary code is not good in the long run.
>
> Have you ever written a #shouldnt: test ? I have never done so !
>
> I took a Seaside image and looked at the senders of #shouldnt: and friends. There are that many from Seaside and I haven’t seen any good example that could not be written otherwise.
>
> My 2c.

They are important :)
Would be good to make a pass on TAssertable too.
I will add a discussion and a big warning in the SUnit Chapter for the next release of the PBE.
Because Camillo is right. shouldnt:raise: Error should not be used.

Stef