[Merge] lp:~sergiusens/account-polld/twitter_avatar into lp:account-polld
Roberto Alsina
roberto.alsina at canonical.com
Sat Jul 26 21:17:54 UTC 2014
Review: Approve
Other than the inline comment which is just a doubt, +1
Diff comments:
> === modified file 'plugins/plugins.go'
> --- plugins/plugins.go 2014-07-25 20:09:20 +0000
> +++ plugins/plugins.go 2014-07-26 21:03:48 +0000
> @@ -19,10 +19,20 @@
>
> import (
> "errors"
> + "io"
> + "net/http"
> + "os"
> + "path"
> + "path/filepath"
>
> "launchpad.net/account-polld/accounts"
> + "launchpad.net/go-xdg/v0"
> )
>
> +func init() {
> + cmdName = os.Args[0]
> +}
> +
> // Plugin is an interface which the plugins will adhere to for the poll
> // daemon to interact with.
> //
> @@ -131,6 +141,8 @@
> // the web service reported that the authentication token has expired.
> var ErrTokenExpired = errors.New("Token expired")
>
> +var cmdName string
> +
> // DefaultSound returns the path to the default sound for a Notification
> func DefaultSound() string {
> return "/usr/share/sounds/ubuntu/notifications/Slick.ogg"
> @@ -140,3 +152,47 @@
> func DefaultVibration() *Vibrate {
> return &Vibrate{Duration: 200}
> }
> +
> +func DownloadAvatar(pluginName, url string) (string, error) {
I keep getting confused as to whether to return err, result or result, err in go.
If we want to check the error, it makes sense to return error first so it's harder to ignore.
> + filePart := filepath.Join(cmdName, "avatars", pluginName, path.Base(url))
> + if file, err := xdg.Cache.Find(filePart); err == nil {
> + return file, nil
> + }
> +
> + file, err := xdg.Cache.Ensure(filePart)
> + if err != nil {
> + return "", err
> + }
> +
> + if err := download(file, url); err != nil {
> + return "", err
> + }
> +
> + return file, nil
> +}
> +
> +func download(file, url string) (err error) {
> + defer func() {
> + if err != nil {
> + os.Remove(file)
> + }
> + }()
> +
> + out, err := os.Create(file)
> + if err != nil {
> + return err
> + }
> + defer out.Close()
> +
> + resp, err := http.Get(url)
> + if err != nil {
> + return err
> + }
> + defer resp.Body.Close()
> +
> + if _, err := io.Copy(out, resp.Body); err != nil {
> + return err
> + }
> +
> + return nil
> +}
>
> === modified file 'plugins/twitter/twitter.go'
> --- plugins/twitter/twitter.go 2014-07-25 19:01:28 +0000
> +++ plugins/twitter/twitter.go 2014-07-26 21:03:48 +0000
> @@ -32,7 +32,10 @@
>
> var baseUrl, _ = url.Parse("https://api.twitter.com/1.1/")
>
> -const twitterIcon = "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.developer.webapps.webapp-twitter/twitter.png"
> +const (
> + twitterIcon = "/usr/share/click/preinstalled/.click/users/@all/com.ubuntu.developer.webapps.webapp-twitter/twitter.png"
> + pluginName = "twitter"
> +)
>
> const (
> maxIndividualStatuses = 2
> @@ -107,13 +110,17 @@
>
> pushMsg := []plugins.PushMessage{}
> for _, s := range statuses {
> + icon, err := plugins.DownloadAvatar(pluginName, s.User.Image)
> + if err != nil {
> + icon = twitterIcon
> + }
> pushMsg = append(pushMsg, plugins.PushMessage{
> Notification: plugins.Notification{
> Card: &plugins.Card{
> Summary: fmt.Sprintf("@%s mentioned you", s.User.ScreenName),
> Body: s.Text,
> Actions: []string{fmt.Sprintf("http://mobile.twitter.com/%s/statuses/%d", s.User.ScreenName, s.Id)},
> - Icon: twitterIcon,
> + Icon: icon,
> Persist: true,
> Popup: true,
> },
> @@ -180,13 +187,17 @@
>
> pushMsg := []plugins.PushMessage{}
> for _, m := range dms {
> + icon, err := plugins.DownloadAvatar(pluginName, m.Sender.Image)
> + if err != nil {
> + icon = twitterIcon
> + }
> pushMsg = append(pushMsg, plugins.PushMessage{
> Notification: plugins.Notification{
> Card: &plugins.Card{
> Summary: fmt.Sprintf("@%s sent you a DM", m.Sender.ScreenName),
> Body: m.Text,
> Actions: []string{fmt.Sprintf("http://mobile.twitter.com/%s/messages", m.Sender.ScreenName)},
> - Icon: twitterIcon,
> + Icon: icon,
> Persist: true,
> Popup: true,
> },
>
--
https://code.launchpad.net/~sergiusens/account-polld/twitter_avatar/+merge/228405
Your team Ubuntu Phablet Team is subscribed to branch lp:account-polld.
More information about the Ubuntu-reviews
mailing list