[Merge] lp:~lorn-potter/media-hub/fix-1420728 into lp:media-hub

Jim Hodapp jim.hodapp at canonical.com
Thu Jun 16 13:13:16 UTC 2016


Review: Needs Fixing code

Don't forget to set the commit message and description.

A few fix suggestions inline below.

Diff comments:

> === modified file 'src/core/media/gstreamer/playbin.cpp'
> --- src/core/media/gstreamer/playbin.cpp	2016-06-08 16:54:31 +0000
> +++ src/core/media/gstreamer/playbin.cpp	2016-06-16 00:15:21 +0000
> @@ -639,8 +639,30 @@
>      if (!info)
>          return std::string();
>  
> -    return std::string(g_file_info_get_attribute_string(
> -                info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE));
> +    std::string content_type = g_file_info_get_attribute_string(info.get(), G_FILE_ATTRIBUTE_STANDARD_FAST_CONTENT_TYPE);

This line might be longer than 100 chars as well, please break.

Also, const std::string

> +    if (content_type.empty())
> +    {
> +        return std::string();

The style of the other code is to not use {} around a one line if-statement body. Please remove them.

> +    }
> +
> +    if (content_type == "application/octet-stream") {

'{' bracket should start on its own line.

> +
> +        std::unique_ptr<GFileInfo, void(*)(void *)> full_info(g_file_query_info(file.get(), G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE,

Please break this line at 100 chars or less. That's the general standard we are trying to follow.

> +                                                        G_FILE_QUERY_INFO_NONE,
> +                                                        /* cancellable */ NULL, &error),g_object_unref);
> +
> +        if (!full_info)
> +        {
> +            return std::string();

See comment above.

> +        }
> +
> +        content_type = g_file_info_get_attribute_string(full_info.get(), G_FILE_ATTRIBUTE_STANDARD_CONTENT_TYPE);

This line might be longer than 100 chars as well, please break.

> +        if (content_type.empty())
> +        {
> +            return std::string();

See comment above.

> +        }
> +    }
> +    return content_type;
>  }
>  
>  std::string gstreamer::Playbin::encode_uri(const std::string& uri) const


-- 
https://code.launchpad.net/~lorn-potter/media-hub/fix-1420728/+merge/297580
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.



More information about the Ubuntu-reviews mailing list