flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Harui <aha...@adobe.com>
Subject Re: git commit: [flex-sdk] [refs/heads/develop] - fix up new MXML codegen and Databinding codegen handling
Date Sun, 03 Nov 2013 04:51:06 GMT
Well, the changes you copied into this thread I think are all in Falcon
java files aren't they?  The code checked into the SDK should be gated by
that one null check and some generated code from Falcon.

And the net is that Falcon generated the new codegen and BasicTests
passed.  I'm now debugging my internal customer's huge app.  I think the
SDK changes have been in long enough for Mustella to pass as well.

I'll try doing more local commits, but sometimes with Falcon code when I
tried that I ended up reverting some commits and that also made the check
in hard to read.

-Alex

On 11/2/13 8:38 PM, "Justin Mclean" <justin@classsoftware.com> wrote:

>Hi,
>
>> Yeah, sorry.  This is all for Falcon.  Doesn't affect MXMLC SWFs at all.
>
>Are you 100% sure? I'm not sure and not looked at the code closely but it
>looks like more than a null check to me.
>
>Looks like there quite a few changes in there and there a couple of
>things that look a little odd at first viewing. I'm not familiar with the
>code so it hard to tell what all these changes are actually for.
>
>For instance logic here you have && ! flex SDK and then else || FlexSDK
>shouldn't the second be an &&? Again I don't know and it could be fine it
>just looks odd on first viewing.
>+            if (s.contains(".") && !isFlexSDK)
>            {
>                String[] parts = s.split("\\.");
>                for (String part : parts)
>                    ret.addInstruction(OP_pushstring, part);
>                ret.addInstruction(OP_newarray, parts.length);
>            }
>-            else if (s == null || s.length() == 0)
>+            else if (s == null || s.length() == 0 || isFlexSDK
>
>And in one part this line was removed "for (int i = 0; i <
>node.getChildCount(); i++)" and replaced with something else, but a
>little further down it's left in.
>
>And changes like this just seem mysterious.
>-            propertyCount += 4;
>+            propertyCount += 5;
>
>I would of expected changes like large to be a merge of a branch with at
>least a dozen check in comments. Done like this it makes it quite hard to
>review. Might also be helpful to some discussion on the list about the
>changes and/or some comments in the code.
>
>Thanks,
>Justin


Mime
View raw message