This repository has been archived by the owner. It is now read-only.

functionality + api updates. #17

Open
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
@rhaining
  • If user hits 'cancel' in login screen, modal view controller will be dismissed.
  • Added ability to send invitation to another user.
  • Added ability to logout - clears cookies & request access invalidation from LinkedIn.
  • Updated profile request to get more info.
  • Modified "updateStatus" to use new API method, "person-activities".
  • Modified ResponseParser to better handle XML responses.
Robert Tolar Haining
* If user hits 'cancel' in login screen, modal view controller will b…
…e dismissed.

* Added ability to send invitation to another user.
* Added ability to logout - clears cookies & request access invalidation from LinkedIn.
* Updated profile request to get more info.
* Modified "updateStatus" to use new API method, "person-activities".
* Modified ResponseParser to better handle XML responses.
@sixten

This comment has been minimized.

Show comment
Hide comment
@sixten

sixten Apr 29, 2011

Contributor

Thanks for sending this! Things are a little crazy these days, but we will review these changes some time in the next few weeks.

Contributor

sixten commented Apr 29, 2011

Thanks for sending this! Things are a little crazy these days, but we will review these changes some time in the next few weeks.

@amitbattan

This comment has been minimized.

Show comment
Hide comment
@amitbattan

amitbattan May 2, 2011

@rhaining I have a issue while logout detail in issue #15

I have tried your code but same issue

@rhaining I have a issue while logout detail in issue #15

I have tried your code but same issue

@rhaining

This comment has been minimized.

Show comment
Hide comment
@rhaining

rhaining May 3, 2011

@sixten cool thanks! thought some of the updates/modifications i made might be of use to others out there. Let me know if you run into any issues

rhaining commented May 3, 2011

@sixten cool thanks! thought some of the updates/modifications i made might be of use to others out there. Let me know if you run into any issues

- if (shouldfree)
- [encodedParameters release];
+ //if (shouldfree)
+ // [encodedParameters release];

This comment has been minimized.

@sixten

sixten Jun 2, 2011

Contributor

While I do still intend to merge your changes as soon as I can get out from under for a few days, I'd love to avoid modifying the upstream code, if at all possible. (Especially something folks may already have in their projects.) Can you tell me a little more about this? Is there a path where this string was over- or under-released?

@sixten

sixten Jun 2, 2011

Contributor

While I do still intend to merge your changes as soon as I can get out from under for a few days, I'd love to avoid modifying the upstream code, if at all possible. (Especially something folks may already have in their projects.) Can you tell me a little more about this? Is there a path where this string was over- or under-released?

This comment has been minimized.

@rhaining

rhaining Jun 2, 2011

The object wasn't getting over- or under-released, but i think the way in which was coded was confusing - even xcode's static analyzer thought there could be a problem. By making encodedParameters always autoreleased & getting rid of 'shouldfree', i think confusion is reduced. However, i totally get the hesitation to modify upstream.

Obviously you can drop the formatting- my apologies for committing those changes!

@rhaining

rhaining Jun 2, 2011

The object wasn't getting over- or under-released, but i think the way in which was coded was confusing - even xcode's static analyzer thought there could be a problem. By making encodedParameters always autoreleased & getting rid of 'shouldfree', i think confusion is reduced. However, i totally get the hesitation to modify upstream.

Obviously you can drop the formatting- my apologies for committing those changes!

@sixten

This comment has been minimized.

Show comment
Hide comment
@sixten

sixten Jun 2, 2011

Contributor

Spent most of the day working on this library while waiting on some other things. I haven't added any of the new API methods, but most of the foundational changes from your fork and a couple of others have been integrated. Check out the unstable branch.

Contributor

sixten commented Jun 2, 2011

Spent most of the day working on this library while waiting on some other things. I haven't added any of the new API methods, but most of the foundational changes from your fork and a couple of others have been integrated. Check out the unstable branch.

@rhaining

This comment has been minimized.

Show comment
Hide comment
@rhaining

rhaining Jun 14, 2011

I only see this commit on the unstable branch from my branch:

  • Modified response parser to handle self-closing XML elements.

However, it seems like it's missing part of my code trying to manage self-closing xml elements..

These are still missing from the unstable branch, but it sounds like you're still workin on 'em!

  • If user hits 'cancel' in login screen, modal view controller will be dismissed.
  • Added ability to send invitation to another user.
  • Added ability to logout - clears cookies & request access invalidation from LinkedIn.
  • Modified "updateStatus" to use new API method, "person-activities".

thanks @sixten!

I only see this commit on the unstable branch from my branch:

  • Modified response parser to handle self-closing XML elements.

However, it seems like it's missing part of my code trying to manage self-closing xml elements..

These are still missing from the unstable branch, but it sounds like you're still workin on 'em!

  • If user hits 'cancel' in login screen, modal view controller will be dismissed.
  • Added ability to send invitation to another user.
  • Added ability to logout - clears cookies & request access invalidation from LinkedIn.
  • Modified "updateStatus" to use new API method, "person-activities".

thanks @sixten!

@sixten

This comment has been minimized.

Show comment
Hide comment
@sixten

sixten Jun 14, 2011

Contributor

Yeah, I was able to carve out a day to work on it, but only one.

  • Which part of your response-handling code is missing? Other than some minor stylistic differences, our versions of RDLinkedInResponseParser.m are almost identical.
  • Logout was added to unstable: a78975f. I went a slightly different route, so while I definitely want to give you credit for that, I don't want to imply that any issues with the implementation are your fault, either.
  • The cancellation functionality was also addressed 7c05cfd, but as part of a larger reworking of how the auth controller handles the authorization page. (See also the configuration information in the README.)
  • I haven't made any changes yet to the actual API communication. The biggest reason is that I'm considering whether to switch over to using JSON instead of XML, which should remove a whole host of potential issues around building and parsing XML strings. (And I don't want to write all of that XML code, only to throw it away in the transition.)
  • According to the documentation, the intended successor to the Status Update API is the Share API, while the Network Update API is intended more for "Bob did a thing in my app"-type items. Both are certainly useful, but the intent of -updateStatus is the former.

I really do appreciate the contribution, and I'm sorry again that it's taking me so long to incorporate these changes.

Contributor

sixten commented Jun 14, 2011

Yeah, I was able to carve out a day to work on it, but only one.

  • Which part of your response-handling code is missing? Other than some minor stylistic differences, our versions of RDLinkedInResponseParser.m are almost identical.
  • Logout was added to unstable: a78975f. I went a slightly different route, so while I definitely want to give you credit for that, I don't want to imply that any issues with the implementation are your fault, either.
  • The cancellation functionality was also addressed 7c05cfd, but as part of a larger reworking of how the auth controller handles the authorization page. (See also the configuration information in the README.)
  • I haven't made any changes yet to the actual API communication. The biggest reason is that I'm considering whether to switch over to using JSON instead of XML, which should remove a whole host of potential issues around building and parsing XML strings. (And I don't want to write all of that XML code, only to throw it away in the transition.)
  • According to the documentation, the intended successor to the Status Update API is the Share API, while the Network Update API is intended more for "Bob did a thing in my app"-type items. Both are certainly useful, but the intent of -updateStatus is the former.

I really do appreciate the contribution, and I'm sorry again that it's taking me so long to incorporate these changes.

@rhaining

This comment has been minimized.

Show comment
Hide comment
@rhaining

rhaining Jun 14, 2011

@sixten, so sorry! i'm a newbie at github - just realized that even if you're on an unstable branch, the '.git' link in the box grabs the stable by default. I pulled down the right branch this time. Looks good!

No worries at all on credit - happy to have better/clearer code out there!

thanks!

-rob

@sixten, so sorry! i'm a newbie at github - just realized that even if you're on an unstable branch, the '.git' link in the box grabs the stable by default. I pulled down the right branch this time. Looks good!

No worries at all on credit - happy to have better/clearer code out there!

thanks!

-rob

@rhaining rhaining closed this Jun 14, 2011

@rhaining rhaining reopened this Jun 14, 2011

@rhaining

This comment has been minimized.

Show comment
Hide comment
@rhaining

rhaining Jun 14, 2011

oops sorry bout the close/reopen!

oops sorry bout the close/reopen!

@sixten

This comment has been minimized.

Show comment
Hide comment
@sixten

sixten Jun 14, 2011

Contributor

No worries. I'm pretty new to Git myself. FWIW, cloning the repository gets you everything: an exact replica of the entire thing with all of its history. By default, it'll only set up one local branch, but the rest are there, and git fetch origin (assuming the default names) will get all of the activity, on whatever branch it happens.

Contributor

sixten commented Jun 14, 2011

No worries. I'm pretty new to Git myself. FWIW, cloning the repository gets you everything: an exact replica of the entire thing with all of its history. By default, it'll only set up one local branch, but the rest are there, and git fetch origin (assuming the default names) will get all of the activity, on whatever branch it happens.

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