Yelp / parcelgen

Helpful tool to make data objects easier for Android
Other
65 stars 35 forks source link

Added support for arrays to Parcelgen #9

Closed maltzj closed 10 years ago

maltzj commented 11 years ago

Hey Android Champions, I totally forgot that this change was sitting around in my Github. Basically I got fed up a while back that Parcelgen couldn't properly handle primitive arrays, so I made it handle primitive arrays. If you want me to list the full set of tests that I did just let me know and I'll run through them all.

greggiacovelli commented 11 years ago

( I don't have commit access anymore), but +1

hackyon commented 11 years ago

Hey maltz - sorry I haven't had the time to take a closer look at this. It's been a busy week cause of the preparation for Burning Man and also the deadline for the project I'm working on.

I'll take a closer look at this once I get back from Burning Man (in 1.5 week or so).

maltzj commented 11 years ago

No worries. It's not a high priority fix at all, I just didn't want it to languish in Github limbo forever. Good luck finishing NAME REDACTED :)

maltzj commented 11 years ago

poke

lxdvs commented 11 years ago

HI GUYS

Pretz commented 11 years ago

HOW'S IT GOING MAN

greggiacovelli commented 11 years ago

GUYZ! LOOKS LIKE WE GETTING THE BAND BACK TOGETHER!

On Thu, Sep 19, 2013 at 7:48 AM, Alex Pretzlav notifications@github.comwrote:

HOW'S IT GOING MAN

— Reply to this email directly or view it on GitHubhttps://github.com/Yelp/parcelgen/pull/9#issuecomment-24744364 .

hackyon commented 11 years ago

I gave you a code review, don't worry about needing to respond (enjoy yourself in HK!) anytime soon. We don't really need this feature on our end so it's really not a big deal. Also, I'm quite swamped with work anyways so it usually takes me a while to respond =P

lxdvs commented 10 years ago

Hey this should go in, we're booting up using parcelgen at airbnb.

Pretz commented 10 years ago

:heart:

hackyon commented 10 years ago

w00t!

On Mon, Nov 11, 2013 at 9:54 AM, Alex Pretzlav notifications@github.comwrote:

[image: :heart:]

— Reply to this email directly or view it on GitHubhttps://github.com/Yelp/parcelgen/pull/9#issuecomment-28222814 .

maltzj commented 10 years ago

Is there anything else you need from me in terms of fixes Don? I'd prefer to get all this squared away before finals start over here.

hackyon commented 10 years ago

I don't think the issues in the code review has been addressed yet, can u take a look at them?

On 2013-11-12, at 2:20 AM, maltzj notifications@github.com wrote:

Is there anything else you need from me in terms of fixes Don? I'd prefer to get all this squared away before finals start over here.

— Reply to this email directly or view it on GitHub.

maltzj commented 10 years ago

I thought I hit all of them with my second push (the one right about Alex D's comment). What am I missing?

I only see one which you raised which doesn't now discuss an outdated diff, and that one shouldn't be fixed as far as I can tell.

hackyon commented 10 years ago

I was expecting github to email me about the changes in the diff, but they didn't so I must have missed them. I'll take a look over the next couple of days (although there's a hackathon happening this Friday and things might get hairy...)

hackyon commented 10 years ago

hey this looks really good, just 1 minor comment about the uptab (fix n ship).

can you plz also document some of the testing you've done in this ticket when u have the time?

maltzj commented 10 years ago

To test all of these I added arrays to the Location object. I modified the Yelp! response to respond with the proper additional fields. Lastly, after creating a mock search result, I selected the changed search result in the BusinessesActivity and opened it in the BusinessActivity. As a result, both the JSON reading and parceling mechanisms were tested.

Boolean array: Passed in with zero elements (length was correct) Passed in with one element (length was correct, element was correct) Passed in with two elements (lenth was correct, elements were correct) Parsed a JSON array with the wrong type and got an error Double array: Passed in with one double element (length was correct, element was correct) Passed in with one int element (length was correct, element was correctly promoted) Passed in with two elements (length was correct, element was correct) Float array: Passed in with one int element (length was correct, element was correct) Passed in with a mix of int and float elements (length was correct, elements were correct) Long array: Passed in with one int element (length was correct, element was correct) Passed in with two elements (length was correct, elements were correct) Passed in a double element to the array (length was correct, element was cut off) int array Passed in with one int element (length was correct, element was correct) passed in with two elements (length was correct, elements were correct) passed in with a double element (element was properly truncated) created no array and passed objects around, everything worked byte array Passed in with one element(length was correct and values were correct) Passed in with two elements (length was correct and values were correct) String array pass in with number (displays the number as a string) pass in with zero (displays no length) pass in with one (displays the proper length and the one string) pass in with two (displays the proper length and strings) pass in with empty string Object array: Created a test object Name.json for this which had one integer field: testField Gave no array in response (array was null in Location) Gave one entry (length was correct and field had the right value) Gave two entires (length was correct and field had the right values)

hackyon commented 10 years ago

Super, thanks maltz for making parcelgen more awesome!

lxdvs commented 10 years ago

For the record, we have been using these changes extensively at Airbnb and they look good.

maltzj commented 10 years ago

I'm curious, are either of you all building with Gradle? I made something very basic for Gradle integration (basically autogenerate any unbuilt/updated parcels at compile time) and was wondering if someone who actually knows Gradle could take a look at what I made and tell me if I did it right.

lxdvs commented 10 years ago

If this is directed at me, the answer is no. We use Renderscript heavily, which precludes us from using Gradle for the time being.