For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

View: New views
9 Messages — Rating Filter:   Alert me  

For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by André Bargull :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Change 20080708-bargull-rwY by bargull@dell--p4--2-53 on 2008-07-08 20:14:02
in /home/Admin/src/svn/openlaszlo/trunk
for http://svn.openlaszlo.org/openlaszlo/trunk

Summary: replication-manager takes datapath-slot in swf9

New Features:

Bugs Fixed: LPP-6539

Technical Reviewer: ptw
QA Reviewer: hminsky
Doc Reviewer: (pending)

Documentation:

Release Notes:

Details:
moved some code and learned that swf9 initializes _all_ class members in
the constructor (in constrast to a prototype based system).


Tests:
attached at bugreport

Files:
M WEB-INF/lps/lfc/data/LzReplicationManager.lzs
M WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs

Changeset:
http://svn.openlaszlo.org/openlaszlo/patches/20080708-bargull-rwY.tar




Re: For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by Henry Minsky-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

So was the failure happening because the values that were getting set
in the constructor were getting
reinitialized during the super() call?



On Wed, Jul 23, 2008 at 5:45 PM, André Bargull <a.bargull@...> wrote:

> Change 20080708-bargull-rwY by bargull@dell--p4--2-53 on 2008-07-08 20:14:02
> in /home/Admin/src/svn/openlaszlo/trunk
> for http://svn.openlaszlo.org/openlaszlo/trunk
>
> Summary: replication-manager takes datapath-slot in swf9
>
> New Features:
>
> Bugs Fixed: LPP-6539
>
> Technical Reviewer: ptw
> QA Reviewer: hminsky
> Doc Reviewer: (pending)
>
> Documentation:
>
> Release Notes:
>
> Details:
> moved some code and learned that swf9 initializes _all_ class members in the
> constructor (in constrast to a prototype based system).
>
>
> Tests:
> attached at bugreport
>
> Files:
> M WEB-INF/lps/lfc/data/LzReplicationManager.lzs
> M WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs
>
> Changeset:
> http://svn.openlaszlo.org/openlaszlo/patches/20080708-bargull-rwY.tar
>
>
>
>



--
Henry Minsky
Software Architect
hminsky@...


Re: For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by André Bargull :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

Yep.


On 7/24/2008 12:03 AM, Henry Minsky wrote:

> So was the failure happening because the values that were getting set
> in the constructor were getting
> reinitialized during the super() call?
>
>
>
> On Wed, Jul 23, 2008 at 5:45 PM, André Bargull <a.bargull@...> wrote:
>  
>> Change 20080708-bargull-rwY by bargull@dell--p4--2-53 on 2008-07-08 20:14:02
>> in /home/Admin/src/svn/openlaszlo/trunk
>> for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: replication-manager takes datapath-slot in swf9
>>
>> New Features:
>>
>> Bugs Fixed: LPP-6539
>>
>> Technical Reviewer: ptw
>> QA Reviewer: hminsky
>> Doc Reviewer: (pending)
>>
>> Documentation:
>>
>> Release Notes:
>>
>> Details:
>> moved some code and learned that swf9 initializes _all_ class members in the
>> constructor (in constrast to a prototype based system).
>>
>>
>> Tests:
>> attached at bugreport
>>
>> Files:
>> M WEB-INF/lps/lfc/data/LzReplicationManager.lzs
>> M WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs
>>
>> Changeset:
>> http://svn.openlaszlo.org/openlaszlo/patches/20080708-bargull-rwY.tar
>>
>>
>>
>>
>>    
>
>
>
>  

Re: For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by Henry Minsky-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

approved!

We had better check all our class constructors code for  this screw
case in any cases where the super call is not the first call
in the constructor.

On Wed, Jul 23, 2008 at 6:17 PM, André Bargull <a.bargull@...> wrote:

> Yep.
>
>
> On 7/24/2008 12:03 AM, Henry Minsky wrote:
>>
>> So was the failure happening because the values that were getting set
>> in the constructor were getting
>> reinitialized during the super() call?
>>
>>
>>
>> On Wed, Jul 23, 2008 at 5:45 PM, André Bargull <a.bargull@...>
>> wrote:
>>
>>>
>>> Change 20080708-bargull-rwY by bargull@dell--p4--2-53 on 2008-07-08
>>> 20:14:02
>>> in /home/Admin/src/svn/openlaszlo/trunk
>>> for http://svn.openlaszlo.org/openlaszlo/trunk
>>>
>>> Summary: replication-manager takes datapath-slot in swf9
>>>
>>> New Features:
>>>
>>> Bugs Fixed: LPP-6539
>>>
>>> Technical Reviewer: ptw
>>> QA Reviewer: hminsky
>>> Doc Reviewer: (pending)
>>>
>>> Documentation:
>>>
>>> Release Notes:
>>>
>>> Details:
>>> moved some code and learned that swf9 initializes _all_ class members in
>>> the
>>> constructor (in constrast to a prototype based system).
>>>
>>>
>>> Tests:
>>> attached at bugreport
>>>
>>> Files:
>>> M WEB-INF/lps/lfc/data/LzReplicationManager.lzs
>>> M WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs
>>>
>>> Changeset:
>>> http://svn.openlaszlo.org/openlaszlo/patches/20080708-bargull-rwY.tar
>>>
>>>
>>>
>>>
>>>
>>
>>
>>
>>
>



--
Henry Minsky
Software Architect
hminsky@...


Re: For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by P T Withington-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

In fact, in JS2, you will not have the option of calling super at  
random points in your constructor; it will automatically be called  
first.  The reason is, you should not be touching the `this` object  
until it is at least a fully initialized member of your superclass.  
In JS2, the only thing you can do in a constructor is to specify how  
your constructor's arguments are mapped to your superclass  
constructors parameters.  The syntax is something like:

   function MyClass(a, b, c) : super(b, a) { ... }

We should eventually support that in our parser (and prohibit super  
calls in the body of a constructor).

Rather than move the inits completely out of the constructor and in to  
the `construct` method, why not just move them to after the super call?

There's a bunch of changes in here that are not related to the title  
of the patch.  They look ok, but maybe you should at least make a  
comment about them in your change message?

Otherwise, approved.

On 2008-07-23, at 17:45EDT, André Bargull wrote:

> Change 20080708-bargull-rwY by bargull@dell--p4--2-53 on 2008-07-08  
> 20:14:02
> in /home/Admin/src/svn/openlaszlo/trunk
> for http://svn.openlaszlo.org/openlaszlo/trunk
>
> Summary: replication-manager takes datapath-slot in swf9
>
> New Features:
>
> Bugs Fixed: LPP-6539
>
> Technical Reviewer: ptw
> QA Reviewer: hminsky
> Doc Reviewer: (pending)
>
> Documentation:
>
> Release Notes:
>
> Details:
> moved some code and learned that swf9 initializes _all_ class  
> members in the constructor (in constrast to a prototype based system).
>
>
> Tests:
> attached at bugreport
>
> Files:
> M WEB-INF/lps/lfc/data/LzReplicationManager.lzs
> M WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs
>
> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20080708-bargull-rwY.tar
>
>
>



Re: For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by André Bargull :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

> We should eventually support that in our parser (and prohibit super
> calls in the body of a constructor).
Is there already a JIRA-ticket for this, or will you create one?

> Rather than move the inits completely out of the constructor and in to
> the `construct` method, why not just move them to after the super call?
That won't work, because the LzNode constructor calls its
construct-method, so everything after the super-call in the constructor
will happen after LzNode#construct() has been called. Which means the
LzDatapath "take datapath-slot of immediateparent"-logic did already happen.

> There's a bunch of changes in here that are not related to the title
> of the patch.  They look ok, but maybe you should at least make a
> comment about them in your change message?
- added typing for LzReplicationManager#getCloneForNode(..)
- added typing for LzReplicationManager#setVisible(..)
- removed LzReplicationManager#op_orderf, because it was superfluous
(the same information as in LzReplicationManager#orderpath)
- renamed LzReplicationManager#comp_orderf to
LzReplicationManager#comparator
- moved LzReplicationManager#mergesort() (simply for legibility and
consistency, first __LZHandle*Nodes() and then helper-methods)
- added doc LzReplicationManager#mergesort()

(I've added these information to the change textfile).

On 7/24/2008 2:37 PM, P T Withington wrote:

> In fact, in JS2, you will not have the option of calling super at
> random points in your constructor; it will automatically be called
> first.  The reason is, you should not be touching the `this` object
> until it is at least a fully initialized member of your superclass.  
> In JS2, the only thing you can do in a constructor is to specify how
> your constructor's arguments are mapped to your superclass
> constructors parameters.  The syntax is something like:
>
>   function MyClass(a, b, c) : super(b, a) { ... }
>
> We should eventually support that in our parser (and prohibit super
> calls in the body of a constructor).
>
> Rather than move the inits completely out of the constructor and in to
> the `construct` method, why not just move them to after the super call?
>
> There's a bunch of changes in here that are not related to the title
> of the patch.  They look ok, but maybe you should at least make a
> comment about them in your change message?
>
> Otherwise, approved.
>
> On 2008-07-23, at 17:45EDT, André Bargull wrote:
>
>> Change 20080708-bargull-rwY by bargull@dell--p4--2-53 on 2008-07-08
>> 20:14:02
>> in /home/Admin/src/svn/openlaszlo/trunk
>> for http://svn.openlaszlo.org/openlaszlo/trunk
>>
>> Summary: replication-manager takes datapath-slot in swf9
>>
>> New Features:
>>
>> Bugs Fixed: LPP-6539
>>
>> Technical Reviewer: ptw
>> QA Reviewer: hminsky
>> Doc Reviewer: (pending)
>>
>> Documentation:
>>
>> Release Notes:
>>
>> Details:
>> moved some code and learned that swf9 initializes _all_ class members
>> in the constructor (in constrast to a prototype based system).
>>
>>
>> Tests:
>> attached at bugreport
>>
>> Files:
>> M WEB-INF/lps/lfc/data/LzReplicationManager.lzs
>> M WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs
>>
>> Changeset:
>> http://svn.openlaszlo.org/openlaszlo/patches/20080708-bargull-rwY.tar
>>
>>
>>
>
>

Re: For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by P T Withington-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On 2008-07-24, at 08:54EDT, André Bargull wrote:

>> We should eventually support that in our parser (and prohibit super  
>> calls in the body of a constructor).
> Is there already a JIRA-ticket for this, or will you create one?

Need one.

>> Rather than move the inits completely out of the constructor and in  
>> to the `construct` method, why not just move them to after the  
>> super call?
> That won't work, because the LzNode constructor calls its construct-
> method, so everything after the super-call in the constructor will  
> happen after LzNode#construct() has been called. Which means the  
> LzDatapath "take datapath-slot of immediateparent"-logic did already  
> happen.

Ah.  Right.

>> There's a bunch of changes in here that are not related to the  
>> title of the patch.  They look ok, but maybe you should at least  
>> make a comment about them in your change message?
> - added typing for LzReplicationManager#getCloneForNode(..)
> - added typing for LzReplicationManager#setVisible(..)
> - removed LzReplicationManager#op_orderf, because it was superfluous  
> (the same information as in LzReplicationManager#orderpath)
> - renamed LzReplicationManager#comp_orderf to  
> LzReplicationManager#comparator
> - moved LzReplicationManager#mergesort() (simply for legibility and  
> consistency, first __LZHandle*Nodes() and then helper-methods)
> - added doc LzReplicationManager#mergesort()
>
> (I've added these information to the change textfile).

Great.  Please check in!

> On 7/24/2008 2:37 PM, P T Withington wrote:
>> In fact, in JS2, you will not have the option of calling super at  
>> random points in your constructor; it will automatically be called  
>> first.  The reason is, you should not be touching the `this` object  
>> until it is at least a fully initialized member of your  
>> superclass.  In JS2, the only thing you can do in a constructor is  
>> to specify how your constructor's arguments are mapped to your  
>> superclass constructors parameters.  The syntax is something like:
>>
>>  function MyClass(a, b, c) : super(b, a) { ... }
>>
>> We should eventually support that in our parser (and prohibit super  
>> calls in the body of a constructor).
>>
>> Rather than move the inits completely out of the constructor and in  
>> to the `construct` method, why not just move them to after the  
>> super call?
>>
>> There's a bunch of changes in here that are not related to the  
>> title of the patch.  They look ok, but maybe you should at least  
>> make a comment about them in your change message?
>>
>> Otherwise, approved.
>>
>> On 2008-07-23, at 17:45EDT, André Bargull wrote:
>>
>>> Change 20080708-bargull-rwY by bargull@dell--p4--2-53 on  
>>> 2008-07-08 20:14:02
>>> in /home/Admin/src/svn/openlaszlo/trunk
>>> for http://svn.openlaszlo.org/openlaszlo/trunk
>>>
>>> Summary: replication-manager takes datapath-slot in swf9
>>>
>>> New Features:
>>>
>>> Bugs Fixed: LPP-6539
>>>
>>> Technical Reviewer: ptw
>>> QA Reviewer: hminsky
>>> Doc Reviewer: (pending)
>>>
>>> Documentation:
>>>
>>> Release Notes:
>>>
>>> Details:
>>> moved some code and learned that swf9 initializes _all_ class  
>>> members in the constructor (in constrast to a prototype based  
>>> system).
>>>
>>>
>>> Tests:
>>> attached at bugreport
>>>
>>> Files:
>>> M WEB-INF/lps/lfc/data/LzReplicationManager.lzs
>>> M WEB-INF/lps/lfc/data/LzLazyReplicationManager.lzs
>>>
>>> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20080708-bargull-rwY.tar
>>>
>>>
>>>
>>
>>



Re: For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by Donald Anderson-3 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Jul 24, 2008, at 9:34 AM, P T Withington wrote:

On 2008-07-24, at 08:54EDT, André Bargull wrote:

We should eventually support that in our parser (and prohibit super calls in the body of a constructor).
Is there already a JIRA-ticket for this, or will you create one?

Need one.

Does http://www.openlaszlo.org/jira/browse/LPP-5758 already cover this?

- Don

--

Don Anderson
Java/C/C++, Berkeley DB, systems consultant

voice: 617-547-7881
email: dda@...
www: http://www.ddanderson.com




Re: For Review: Change 20080708-bargull-rwY Summary: replication-manager takes datapath-slot in swf9

by P T Withington-2 :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On 2008-07-24, at 10:30EDT, Donald Anderson wrote:

> On Jul 24, 2008, at 9:34 AM, P T Withington wrote:
>
>> On 2008-07-24, at 08:54EDT, André Bargull wrote:
>>
>>>> We should eventually support that in our parser (and prohibit  
>>>> super calls in the body of a constructor).
>>> Is there already a JIRA-ticket for this, or will you create one?
>>
>> Need one.
>
>
> Does http://www.openlaszlo.org/jira/browse/LPP-5758 already cover  
> this?

It does if we implement the "bonus" part.  :)