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 >> >> >> >> >> >>
