poll-daemon

Merge lp://qastaging/~jamesh/account-polld/twitter-plugin into lp://qastaging/~phablet-team/account-polld/trunk

  1. twitter-plugin
  2. Merge into trunk
Proposed by James Henstridge
Status: Merged
Merged at revision: 11
Proposed branch: lp://qastaging/~jamesh/account-polld/twitter-plugin
Merge into: lp://qastaging/~phablet-team/account-polld/trunk
Diff against target: 1461 lines (+1383/-9)
8 files modified
cmd/account-polld/main.go (+3/-2)
plugins/facebook/facebook_test.go (+7/-7)
plugins/twitter/oauth/README.markdown (+22/-0)
plugins/twitter/oauth/examples_test.go (+54/-0)
plugins/twitter/oauth/oauth.go (+456/-0)
plugins/twitter/oauth/oauth_test.go (+172/-0)
plugins/twitter/twitter.go (+214/-0)
plugins/twitter/twitter_test.go (+455/-0)
To merge this branch: bzr merge lp://qastaging/~jamesh/account-polld/twitter-plugin
Reviewer Review Type Date Requested Status
Sergio Schvezov Pending
Review via email: mp+227313@code.qastaging.launchpad.net

Commit message

Add the Twitter polling plugin.

Description of the change

Add the Twitter plugin, producing notifications for mentions and direct messages. Both of these features work with the permissions provided by the token from the "twitter-microblog" service.

Incremental results are returned using the since_id API option.

OAuth 1.0a signing is done using the external github.com/garyburd/go-oauth/oauth library, since Twitter requires HMAC-SHA1 request signing, which is decidedly non-trivial (this is also the reason why there are 4 auth strings in the accounts.AuthData struct).

To post a comment you must log in.
Revision history for this message
Sergio Schvezov (sergiusens) wrote :

On viernes 18 de julio de 2014 08h'49:24 ART, James Henstridge wrote:
> James Henstridge has proposed merging
> lp:~jamesh/account-polld/twitter-plugin into lp:account-polld.
>
> Commit message:
> Add the Twitter polling plugin.
>
> Requested reviews:
> Sergio Schvezov (sergiusens)
>
> For more details, see:
> https://code.launchpad.net/~jamesh/account-polld/twitter-plugin/+merge/227313

110 + // Resolve path relative to Graph API base URL, and add access token
this seems to be a stray comment.

250 + type user struct {
seems to be missing a gofmt/goimports call

97 + lastMentionId int64
98 + lastDirectMessageId int64
just out of curiosity, why are these int64 instead of uint64? Is there a
chance for negative Ids?

I'll do more thorough review in the morning.

lp://qastaging/~jamesh/account-polld/twitter-plugin updated
14. By James Henstridge

Fix up comment, and run through gofmt.

Revision history for this message
James Henstridge (jamesh) wrote :

I've fixed the comment and run the code through gofmt.

The Twitter web site recommended using 64 bit integers. It didn't say one way or the other about signed/unsigned, and the two Go Twitter libraries linked from https://dev.twitter.com/docs/twitter-libraries seem to have chosen differently.

It's hard to tell whether this will be a problem, since (a) the range of 64-bit integers is so large and (b) they'd need to come up with a solution for languages without unsigned types like Java before they got to that point anyway.

Revision history for this message
Sergio Schvezov (sergiusens) wrote :

Two more things; Lucio told me it would be good to setup the Icon in the
bubble; he said it was ok to use the one provided in the click app for now;
that is located in:

/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.developer.webapps.webapp-twitter/twitter.png

I will switch to loading than from the push helper once this lands (as the
helper would be part of the click itself).

The other thing is; can we inline/embed the oauth package into this trunk
as part of this merge?

PS: totally unrelated, but I can't comment on this MP.

lp://qastaging/~jamesh/account-polld/twitter-plugin updated
15. By James Henstridge

Add icon to Twitter messages.

16. By James Henstridge

Embed the OAuth 1 library, since we don't have it packaged.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches