Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

As3Vanilla - added caching and Enum support #8

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

natami
Copy link

@natami natami commented Dec 29, 2013

Hi, Jonny!

I previously asked, if you could add Enum support to our As3Vanilla project - knowing you are probably very busy, I added support for Java style Enums and implemented InjectionMap caching support.

Hope you can find the time to merge my changes into your project.

Regards,
Soren Jepsen

Upgraded as3commons-lang and as3commons-reflect.
Upgraded FlexUnit version.
…interface.

Fixed a couple of bugs, and refactored some code.
Added InjectionCaching support.
Added related unit-tests to Caching classes.
Refactored a bit and removed some unused code.
Added furter unit-tests.
@natami
Copy link
Author

natami commented Feb 28, 2014

Hi, Jonny!

I fixed a couple of errors etc. that popped up during daily use of your As3Vanilla project.

@matej-snivam
Copy link

How to use this?

@natami
Copy link
Author

natami commented Jul 3, 2014

Take a look at the docs at:
https://github.com/natami/as3-vanilla

@matej-snivam
Copy link

nice, thx

Matej Šimunić

On 4 July 2014 00:58, natami [email protected] wrote:

Take a look at the docs at:
https://github.com/natami/as3-vanilla


Reply to this email directly or view it on GitHub
#8 (comment).

@@ -0,0 +1,44 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove and add to .gitignore

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still need to git rm this file.

Removed authorship comment.
Added indent to IEnum.as
@natami
Copy link
Author

natami commented Jul 6, 2014

Fixed :)

@jonnyreeves
Copy link
Owner

Sorry for not getting back to you sooner natami - it's been quite a crazy year so far, new house and my wife has just had our first child; things are only starting to settle down now :)

Thanks for the PR; you've put a lot of work in here! Could you please revisit the formatting and ensure that you use the same style as the original code, this will make it much easier to review.

Thanks again
Jonny.

@natami
Copy link
Author

natami commented Jul 6, 2014

Hi, Jonny!

That’s quite all right - other stuff is just more important than keeping a project in sync on Github ;)

I’ll adjust the formatting in Intellij, so i can follow your style.

And congrats on the house and family ;)

Regards,
Søren.

On 06 Jul 2014, at 21:29, John Reeves [email protected] wrote:

Sorry for not getting back to you sooner natami - it's been quite a crazy year so far, new house and my wife has just had our first child; things are only starting to settle down now :)

Thanks for the PR; you've put a lot of work in here! Could you please revisit the formatting and ensure that you use the same style as the original code, this will make it much easier to review.

Thanks again
Jonny.


Reply to this email directly or view it on GitHub.

@natami
Copy link
Author

natami commented Jul 6, 2014

Ok, fixed the remaining formatting issues.


<target name="init">
<target name="init">
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix tabbing here (2 tabs, not 1).

@jonnyreeves
Copy link
Owner

Hey @natami, sorry to be a pain, but could you use tabs, not spaces?

@natami
Copy link
Author

natami commented Jul 6, 2014

Quite allright.

Always used intelliJ defaults. Should be fixed now.

result.push(methodName);
}
return result;
return getNames(_methods);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need getMethodNames and getNames? Oh I see, you're removing the duplication between these methods - fair enough.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey John, as a maintainer of public repo, you can get a free copy of the
IntelliJ. Maybe you qualify. It is a great IDE

Matej Šimunić

On 6 July 2014 23:27, John Reeves [email protected] wrote:

In src/org/osflash/vanilla/InjectionMap.as:

    public function getMethodsNames() : Array
    {
  •       const result : Array = [];
    
  •       for (var methodName : String in _methods) {
    
  •           result.push(methodName);
    
  •       }
    
  •       return result;
    
  •       return getNames(_methods);
    

Why do we need getMethodNames and getNames?


Reply to this email directly or view it on GitHub
https://github.com/jonnyreeves/as3-vanilla/pull/8/files#r14577706.

@jonnyreeves
Copy link
Owner

Thanks @natami, there's still a great deal of change; I'm guessing you re-formatted the code based on your IDE's formatting rules? It makes it really hard to diff.

I'm not actually working with flash anymore and don't really plan to add more features to vanilla (and provide support fro them) - do you have any thoughts on taking ownership / maintaining your own fork of this project to provide that support.

@smival
Copy link

smival commented Jul 14, 2014

Hi, thanks for sharing!
Can you suggest some way to solve my issue: I have Array of custom objects.
Of course each Array contains only one objects type.

@natami
Copy link
Author

natami commented Jul 15, 2014

Hi, Smival!

Try changeing the array to a vector.<> type...

Should solve your issue.

  • I will try checking out array support.

@natami
Copy link
Author

natami commented Aug 23, 2014

Hi, Jonny!

Sorry for not getting back to you with a timely answer - work has been crazy over the summer.

As things are settling down and i'm getting a bit more time for personal projects, and i am interested in possibly taking ownership/support over the project - if that is okay with you?

Regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants