changes to core should be treated with care

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

changes to core should be treated with care

Tudor Girba-2
Hi Santiago,

I noticed that you committed a change to Famix-Core.

The comment says that the intention of the commit was to add WMC to
package. This is Ok.

The problem is that in the process, you also:
- deleted FAMIXClass>>isInterface and FAMIXClass>>isInterface:
- introduced null checks in core methods (classScope, packageScope and
namespaceScope)

I am sure that the first one was a mistake. But, for the second one
you should have raise it for discussion because it has to do with
changing the semantics of those methods.

In general, changes to Core classes should be treated with more care
(this is valid for me as well) :)

Cheers,
Doru

--
www.tudorgirba.com
"Every thing has its own flow"
_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: changes to core should be treated with care

Santiago Vidal
Hi,

Sorry, as you said  it was a mistake. I'm going to be more careful with the changes. Alexandre did a merge that fix the problems.
Cheers,
  Santiago


2011/11/23 Tudor Girba <[hidden email]>
Hi Santiago,

I noticed that you committed a change to Famix-Core.

The comment says that the intention of the commit was to add WMC to
package. This is Ok.

The problem is that in the process, you also:
- deleted FAMIXClass>>isInterface and FAMIXClass>>isInterface:
- introduced null checks in core methods (classScope, packageScope and
namespaceScope)

I am sure that the first one was a mistake. But, for the second one
you should have raise it for discussion because it has to do with
changing the semantics of those methods.

In general, changes to Core classes should be treated with more care
(this is valid for me as well) :)

Cheers,
Doru

--
www.tudorgirba.com
"Every thing has its own flow"
_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--
Santiago Vidal

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: changes to core should be treated with care

abergel
In reply to this post by Tudor Girba-2
I have merged.
No idae how these isInterface and isInterface: disappeared.

For the null check, we should have indeed discussed it. We run into error without them. We should try to write some tests for it.

Alexandre


On 23 Nov 2011, at 12:28, Tudor Girba wrote:

> Hi Santiago,
>
> I noticed that you committed a change to Famix-Core.
>
> The comment says that the intention of the commit was to add WMC to
> package. This is Ok.
>
> The problem is that in the process, you also:
> - deleted FAMIXClass>>isInterface and FAMIXClass>>isInterface:
> - introduced null checks in core methods (classScope, packageScope and
> namespaceScope)
>
> I am sure that the first one was a mistake. But, for the second one
> you should have raise it for discussion because it has to do with
> changing the semantics of those methods.
>
> In general, changes to Core classes should be treated with more care
> (this is valid for me as well) :)
>
> Cheers,
> Doru
>
> --
> www.tudorgirba.com
> "Every thing has its own flow"
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.





_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: changes to core should be treated with care

Tudor Girba-2
In reply to this post by Santiago Vidal
Thanks.

However, he did not fix the issues. isInterface: is still removed. And
the nil checks are still there. Please remove them :).

Furthermore, the metrics should be put in the Famix-Extensions package.

Cheers,
Doru


On Wed, Nov 23, 2011 at 4:50 PM, Santiago Vidal
<[hidden email]> wrote:

> Hi,
> Sorry, as you said  it was a mistake. I'm going to be more careful with the
> changes. Alexandre did a merge that fix the problems.
> Cheers,
>   Santiago
>
> 2011/11/23 Tudor Girba <[hidden email]>
>>
>> Hi Santiago,
>>
>> I noticed that you committed a change to Famix-Core.
>>
>> The comment says that the intention of the commit was to add WMC to
>> package. This is Ok.
>>
>> The problem is that in the process, you also:
>> - deleted FAMIXClass>>isInterface and FAMIXClass>>isInterface:
>> - introduced null checks in core methods (classScope, packageScope and
>> namespaceScope)
>>
>> I am sure that the first one was a mistake. But, for the second one
>> you should have raise it for discussion because it has to do with
>> changing the semantics of those methods.
>>
>> In general, changes to Core classes should be treated with more care
>> (this is valid for me as well) :)
>>
>> Cheers,
>> Doru
>>
>> --
>> www.tudorgirba.com
>> "Every thing has its own flow"
>> _______________________________________________
>> Moose-dev mailing list
>> [hidden email]
>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
>
>
> --
> Santiago Vidal
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
>



--
www.tudorgirba.com
"Every thing has its own flow"

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: changes to core should be treated with care

Santiago Vidal
Sorry, we were out for a couple of hours. I had seen that you made the changes.  I'm going to be more careful with the changes next time.
Thank you

2011/11/23 Tudor Girba <[hidden email]>
Thanks.

However, he did not fix the issues. isInterface: is still removed. And
the nil checks are still there. Please remove them :).

Furthermore, the metrics should be put in the Famix-Extensions package.

Cheers,
Doru


On Wed, Nov 23, 2011 at 4:50 PM, Santiago Vidal
<[hidden email]> wrote:
> Hi,
> Sorry, as you said  it was a mistake. I'm going to be more careful with the
> changes. Alexandre did a merge that fix the problems.
> Cheers,
>   Santiago
>
> 2011/11/23 Tudor Girba <[hidden email]>
>>
>> Hi Santiago,
>>
>> I noticed that you committed a change to Famix-Core.
>>
>> The comment says that the intention of the commit was to add WMC to
>> package. This is Ok.
>>
>> The problem is that in the process, you also:
>> - deleted FAMIXClass>>isInterface and FAMIXClass>>isInterface:
>> - introduced null checks in core methods (classScope, packageScope and
>> namespaceScope)
>>
>> I am sure that the first one was a mistake. But, for the second one
>> you should have raise it for discussion because it has to do with
>> changing the semantics of those methods.
>>
>> In general, changes to Core classes should be treated with more care
>> (this is valid for me as well) :)
>>
>> Cheers,
>> Doru
>>
>> --
>> www.tudorgirba.com
>> "Every thing has its own flow"
>> _______________________________________________
>> Moose-dev mailing list
>> [hidden email]
>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
>
>
> --
> Santiago Vidal
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>
>



--
www.tudorgirba.com
"Every thing has its own flow"

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev



--
Santiago Vidal

_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: changes to core should be treated with care

abergel
In reply to this post by Tudor Girba-2
> Furthermore, the metrics should be put in the Famix-Extensions package.

Done, in Famix-Extensions-AlexandreBergel.211

Cheers,
Alexandre

>
>
> On Wed, Nov 23, 2011 at 4:50 PM, Santiago Vidal
> <[hidden email]> wrote:
>> Hi,
>> Sorry, as you said  it was a mistake. I'm going to be more careful with the
>> changes. Alexandre did a merge that fix the problems.
>> Cheers,
>>   Santiago
>>
>> 2011/11/23 Tudor Girba <[hidden email]>
>>>
>>> Hi Santiago,
>>>
>>> I noticed that you committed a change to Famix-Core.
>>>
>>> The comment says that the intention of the commit was to add WMC to
>>> package. This is Ok.
>>>
>>> The problem is that in the process, you also:
>>> - deleted FAMIXClass>>isInterface and FAMIXClass>>isInterface:
>>> - introduced null checks in core methods (classScope, packageScope and
>>> namespaceScope)
>>>
>>> I am sure that the first one was a mistake. But, for the second one
>>> you should have raise it for discussion because it has to do with
>>> changing the semantics of those methods.
>>>
>>> In general, changes to Core classes should be treated with more care
>>> (this is valid for me as well) :)
>>>
>>> Cheers,
>>> Doru
>>>
>>> --
>>> www.tudorgirba.com
>>> "Every thing has its own flow"
>>> _______________________________________________
>>> Moose-dev mailing list
>>> [hidden email]
>>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>>
>>
>>
>> --
>> Santiago Vidal
>>
>> _______________________________________________
>> Moose-dev mailing list
>> [hidden email]
>> https://www.iam.unibe.ch/mailman/listinfo/moose-dev
>>
>>
>
>
>
> --
> www.tudorgirba.com
> "Every thing has its own flow"
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.





_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: changes to core should be treated with care

Stéphane Ducasse
In reply to this post by Tudor Girba-2
what is that?

> - introduced null checks in core methods (classScope, packageScope and
> namespaceScope)



_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev
Reply | Threaded
Open this post in threaded view
|

Re: changes to core should be treated with care

abergel
Cf the other mail

Alexandre


On 23 Nov 2011, at 18:09, Stéphane Ducasse wrote:

> what is that?
>
>> - introduced null checks in core methods (classScope, packageScope and
>> namespaceScope)
>
>
>
> _______________________________________________
> Moose-dev mailing list
> [hidden email]
> https://www.iam.unibe.ch/mailman/listinfo/moose-dev

--
_,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:
Alexandre Bergel  http://www.bergel.eu
^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.






_______________________________________________
Moose-dev mailing list
[hidden email]
https://www.iam.unibe.ch/mailman/listinfo/moose-dev