Thanks for the feedback Alex. I had also received encouragement from Justin a long time ago to contribute, I'm happy that I finally got a chance to do so.
tbh I had not even looked inside manual tests folder yet (there's a lot in there!), but that definitely sounds like a better place for that example, it really is a 'manual test'. it might require the addition of : <arg value="-compiler.binding-event-handler-interface=org.apache.flex.events.IEventDispatcher" /> <arg value="-compiler.binding-event-handler-class=org.apache.flex.events.EventDispatcher" /> <arg value="-compiler.binding-event-handler-event=org.apache.flex.events.Event" /> to the build_example ant file targets, if you didn't already add it In terms of my sleeves, I hope to find something more up there within the next couple of weeks. My regular client work is in a bit of a lull, so I am making use of the time to get familiar with FlexJS. I actually already started work on updates in reflection, and only ended up in bindings because I saw some issues. I will go back to reflection again next, with a goal of getting identical results in swf and js. Some of the stuff I added to the JSSessionModel will help with what I needed to do there anyhow. I will definitely add to or update any relevant existing unit-tests on this next work. cheers, Greg On Tue, Aug 30, 2016 at 7:34 PM, Alex Harui <aha...@adobe.com> wrote: > Hi Greg, > > OK, I looked through your patches and applied them to the develop branch. > I couldn't see anything obviously wrong with it, so great job and thanks > for contributing! I hope you have more up your sleeve. > > FWIW, it would be nice to unit tests in flex-falcon and maybe flex-asjs. > I'm going to move your example to the manual tests folder in a minute here > as I think it belongs there. > > Thanks again, > -Alex > > On 8/28/16, 10:10 PM, "Greg Dove" <greg.d...@gmail.com> wrote: > > >Hey Alex, sorry I wasn't clear. > > > >"Without a test case to step through the code, I have to say that it is a > >bit surprising that you fixed the jx output by fiddling with > >ASCompilationUnit and ClassDirectiveProcessor instead of the emitters." > > > >I meant specifically emitters outside of jx js emitters, and VF2JS was an > >example of one I did not touch. AMD is another. I definitely made changes > >in the FlexJSEmitter stuff. > > > >The ClassDIrectiveProcessor change was just for falcon/swf - I made > >similar > >changes for the js emitters, and implemented the IEventDispatcher > >variation > >as well as it was not currently implemented in jx. > > > >I will be submitting PRs within the next 15 mins. I won't add a lot of > >text > >with the PR - so please ask if you have any questions as to why I did > >things the way I did. Some of it may be a little 'clumsy' perhaps. > > > >cheers > >Greg > > > >On Mon, Aug 29, 2016 at 5:04 PM, Alex Harui <aha...@adobe.com> wrote: > > > >> Hi Greg, > >> > >> Without a test case to step through the code, I have to say that it is a > >> bit surprising that you fixed the jx output by fiddling with > >> ASCompilationUnit and ClassDirectiveProcessor instead of the emitters. > >> > >> I guess I'll wait to see the PR. Maybe it will make more sense then. I > >> guess maybe you needed to make that change to affect the AST? > >> > >> Fundamentally, the compiler creates an AST, then Falcon (the SWF > >>compiler) > >> uses Reducers to reduce the AST to ABC code. FalconJX uses BlockWalkers > >> and Emitters. There are different emitters for different output > >>formats. > >> The vf2js emitters were an attempt to do a straight cross-compile of the > >> existing Flex SDK code. It is not being used by FlexJS. > >> > >> Looking forward to it, > >> -Alex > >> > >> On 8/28/16, 8:51 PM, "Greg Dove" <greg.d...@gmail.com> wrote: > >> > >> >Thanks Alex, > >> > > >> >There are definitely bugs, and I have addressed those that I found in > >>the > >> >testbed example I already made a PR for. The others should beWhether I > >> >have > >> >addressed them appropriately or not I can't say, but if the way I have > >> >done > >> >things is not consistent with the compiler architecture or any general > >> >java > >> >coding standards, then at the very least it should provide clues for > >>you > >> >or > >> >someone else for what needs to be done in a more appropriate way. And > >>if > >> >it > >> >is unclear why I did anything in particular please ask. > >> > > >> >One of the things I ended up doing was to move the 'extends > >> >EventDispatcher' implementation from ASCompilationUnit to > >> >ClassDIrectiveProcessor for falcon and to provide a corresponding > >> >implementation in jx. There seemed to be times when that original > >> >implementation was not being applied when it ought to have been, and it > >> >also seemed like ClassDirectiveProcessor was a more natural home for > >>it, > >> >alongside the other 'implements IEventDispatcher' implementation for > >> >binding support. > >> > > >> > > >> >In terms of your suggestion about asking questions... I appreciate your > >> >intent here, I think most people prefer not to ask stupid questions > >>(even > >> >if there is a culture of 'no such thing as a stupid question'), and I > >> >needed to get my head around the compiler a bit first before asking too > >> >many questions, otherwise most of the questions would probably be > >>stupid > >> >ones about how it works! > >> >I used this exercise to help me understand the basics, and I personally > >> >find this approach is much better for for me to learn anything new over > >> >asking lots of questions (it may not be the most efficient to arrive at > >> >the > >> >knowledge, but it helps me get to a deeper understanding faster I > >>think). > >> >Also, although all you guys are great at responding quickly here, I am > >> >used > >> >to the immediacy of some form of chat, otherwise I am usually already > >> >trying to answer my questions myself, particularly if I am already > >>focused > >> >on it. IRC is a good option here that some OS teams use, because it is > >> >possible to set up some logging (and therefore can be made searchable, > >>if > >> >required). But some people find IRC a bit 'old school' :). > >> > > >> >I will likely ask more questions in the future, but I do have a more > >> >general question now: > >> > > >> >Also, in terms of changing emitter support in jx for js output, I have > >>not > >> >touched anything outside of js emitters - is that usually ok? I don't > >>even > >> >know what vf2js or some of the others are for.... but I do see that > >>some > >> >of > >> >the various emitter classes have duplicated or similar code in parts of > >> >them. I did wonder about whether I was supposed to do anything > >>elsewhere > >> >as > >> >well, but as I did not really understand it, I chose to stop > >>wondering. :) > >> > > >> >cheers, > >> >Greg > >> > > >> > > >> > > >> > > >> >On Sat, Aug 27, 2016 at 5:40 PM, Alex Harui <aha...@adobe.com> wrote: > >> > > >> >> Sounds great! Looking forward to it. > >> >> > >> >> It might be better in the future if you ask more questions as you go > >> >> along. I think the compiler already does IEventDispatcher although I > >> >> wouldn't be surprised if the current code has bugs, so discussing > >>early > >> >> can help make sure you are spending your cycles appropriately. > >> >> > >> >> Thanks, > >> >> -Alex > >> >> > >> >> > >> > >> > >
