Skip to content

FIX: Fixed FLash Client library#2039

Merged
wing328 merged 2 commits intoswagger-api:masterfrom
glederrey:master
Feb 4, 2016
Merged

FIX: Fixed FLash Client library#2039
wing328 merged 2 commits intoswagger-api:masterfrom
glederrey:master

Conversation

@glederrey
Copy link
Copy Markdown
Contributor

Hi,

I fixed the Flash client library. It had multiple problems.. =) I also added a README.txt that explains how to use it.. (It's a bit more difficult to use than Python..)

  • FlashClientCodegen
    • I changed the architecture to make it look like an ActionScript library project.
    • The Number type in as wasn't recognized by Swagger. (You have Double, Float, Integer, etc.) And since it wasn't recognized, Codegen was putting null as default value for Number. It's wrong. Therefore, I changed it to NaN
  • ApiInvoker.as
    • A lot of imports were missing. I added them.
    • Bug at line 102/115 that I fixed
    • I commented a part of the function onApiRequestResult because it was trying to parse the response as a XML. And therefore, we couldn't receive anything if it's in JSON, for example. Therefore, having it RAW is better, for the moment. =)
  • ApiUrlHelper.as
    • Small bug at line 34.
  • api.mustache
    • Here, I simply removed the DefaultValue. It's just because if we put a DefaultValue, then it means for AS that it's an optional parameter. So, if there is other parameters after one parameter with a DefaultValue, AS will trigger an error because it doesn't accept required parameters after optional ones.
  • README.txt
    • Just some explanations.. I hope it will make it more clear how to use it.. I took quite a lot of time to understand how to use it, but I'm not an expert in flash at all.. So, I hope it can help some futur users..

That's everything.. So, I don't really improved this library. But at least, this library generated by Swagger will compile directly and will be ready to be used..

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 4, 2016

Thanks @glederrey 🍻

I would suggest updating the Petstore sample by running ./bin/flash-petstore.sh

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 4, 2016

Also would recommend you to create a new branch for your work (upcoming contribution)

Ref: https://www.git-tower.com/learn/git/ebook/command-line/appendix/best-practices => Use Branches

@glederrey
Copy link
Copy Markdown
Contributor Author

@wing328 You're welcome..
Which branch do you want me to create?? I will update the Petstore Sample..

@glederrey
Copy link
Copy Markdown
Contributor Author

Just a question, in order to change the branch (and this PR), do I have to delete this PR and create a new one with the new branch??

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 4, 2016

For this PR, you can just keep it (master) as it's.

My suggestion is more for moving forward.

@glederrey
Copy link
Copy Markdown
Contributor Author

Okay.. Sorry about that..
I'll do that next time.. =)

I saw that the checks failed for the android client library.. What can I do with this??

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 4, 2016

I've restarted the CI build for that. Let's see how it goes.

@glederrey
Copy link
Copy Markdown
Contributor Author

Okay, thank you

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 4, 2016

CI tests passed. PR merged. Thanks again for the contribution.

wing328 added a commit that referenced this pull request Feb 4, 2016
FIX: Fixed FLash Client library
@wing328 wing328 merged commit 148de83 into swagger-api:master Feb 4, 2016
@glederrey
Copy link
Copy Markdown
Contributor Author

You're welcome.. I hope I'll be able to help you more.. =)

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

Btw, I notice you seem to be using your private account (?) for the commit and therefore the top contributor list is not showing your contribution.

Let me know if you want to redo the change again so that the credit goes back to your public github account glederrey.

@glederrey
Copy link
Copy Markdown
Contributor Author

Hi @wing328,
The swagger-codegen I forked is public.. That's what you're asking??

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

If you look at the commit history for this PR, you will find that the commits are not linked to your Github account @glederrey

That's probably why Github didn't count the 2 commits (contribution) to your Github account @glederrey

@glederrey
Copy link
Copy Markdown
Contributor Author

You're right.. That's weird..

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

The commit log may give you a clue:

commit 5dc67c15e5587aa405d24c0e9cf3beb96ad5ec3c
Author: Gael Lederrey <Gael Lederrey>
Date:   Thu Feb 4 14:33:46 2016 +0100

    ADD: Add Petstore sample for flash

commit 77f9bc3642ae202772e2c4682258b9e47113c3a3
Author: Gael Lederrey <Gael Lederrey>
Date:   Thu Feb 4 14:03:13 2016 +0100

    FIX: Fixed FLash Client library

commit e926c86141a167ebe76afbdff2fdbe463126cb83
Merge: 96f9e93 7b67845
Author: wing328 <wing328hk@gmail.com>
Date:   Thu Feb 4 20:33:46 2016 +0800

    Merge pull request #2035 from wing328/csharp_fix_default_header

    [C#] fix default header in async method and sanitize model name

As you can see, mine has a proper email address linked to it. Yours is just a name. Maybe it has to do with your local git setting.

@glederrey
Copy link
Copy Markdown
Contributor Author

Okay, I see.. There was a problem when I put the username and email..

I changed it for next time.. =)

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

Entirely up to you. If you want to redo it (so that credits go back to @glederrey), just let me know.

@glederrey
Copy link
Copy Markdown
Contributor Author

If we can do it in an easy way.. With pleasure.. =)

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

I can roll back the change for the time being. You can then submit a PR with commits linked to @glederrey .

@glederrey
Copy link
Copy Markdown
Contributor Author

Okay.. Thank you very much

@glederrey
Copy link
Copy Markdown
Contributor Author

Do I have to submit a new PR??

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

Just reverted your PR. Please submit a new one with commits properly linked to your Github account.

@glederrey
Copy link
Copy Markdown
Contributor Author

I will do that.. Thanks again..

@wing328
Copy link
Copy Markdown
Contributor

wing328 commented Feb 5, 2016

Np. Just want to make sure you get the credits for your contribution that you deserved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants