Hi,
Please find below some comments:
On 02/12/11 19:36, gemont igalia com wrote:
[...]
> +/* ========== API ========== */
> +
> +/**
> + * grl_operation_options_new:
> + * @caps: (allow-none): caps that options will have to match. If %NULL, all
> + * options will be accepted.
> + *
> + * Creates a new GrlOperationOptions object.
> + *
> + * Returns: a new GrlOperationOptions instance.
> + */
Are you missing here a "(transfer full):" annotation?
> +GrlOperationOptions *
> +grl_operation_options_new (GrlCaps *caps)
> +{
> + GrlOperationOptions *options = g_object_new (GRL_OPERATION_OPTIONS_TYPE, NULL);
> + if (caps != NULL)
> + options->priv->caps = g_object_ref (caps);
> +
> + return options;
> +}
> +
> +/**
> + * grl_operation_options_obey_caps:
> + * @options: a #GrlOperationOptions instance
> + * @caps: capabilities against which we want to test @options
> + * @supported_options: (out callee-allocates): if not %NULL, will contain a
> + * newly-allocated #GrlOperationOptions instance containing all the values of
> + * @options that match @caps.
> + * @unsupported_options: (out callee-allocates): if not %NULL, will contain a
> + * newly-allocated #GrlOperationOptions instance containing all the values of
> + * @options that do not match @caps.
> + *
> + * Check whether @options obey to @caps.
> + * Optionally provide the options that match (respectively don't match) @caps
> + * in @supported_options (respectively @unsupported_options).
> + * This would typically (but not necessarily) be used with a
> + * #GrlOperationOptions instance that was created with %NULL caps.
> + *
> + * Returns: %TRUE if @options obeys to @caps, %FALSE otherwise.
> + */
Should be "if @options obey", right?
> +gboolean
> +grl_operation_options_obey_caps (GrlOperationOptions *options,
> + GrlCaps *caps,
> + GrlOperationOptions **supported_options,
> + GrlOperationOptions **unsupported_options)
> +{
> + if (supported_options) {
> + *supported_options = grl_operation_options_new (caps);
> +
> + /* these options are always supported */
> + copy_option (options, *supported_options, GRL_OPERATION_OPTION_SKIP);
> + copy_option (options, *supported_options, GRL_OPERATION_OPTION_COUNT);
> + copy_option (options, *supported_options, GRL_OPERATION_OPTION_FLAGS);
> + }
> +
> + if (unsupported_options)
> + *unsupported_options = grl_operation_options_new (NULL);
> +
> + return TRUE;
> +}
> +
> +/**
> + * grl_operation_options_copy:
> + * @options: a #GrlOperationOptions instance
> + *
> + * Returns: (transfer full): a new #GrlOperationOptions instance with its values being copies of
> + * the values of @options.
> + */
> +GrlOperationOptions *
> +grl_operation_options_copy (GrlOperationOptions *options)
> +{
> + GrlOperationOptions *copy = grl_operation_options_new (options->priv->caps);
> +
> + copy_option (options, copy, GRL_OPERATION_OPTION_SKIP);
> + copy_option (options, copy, GRL_OPERATION_OPTION_COUNT);
> + copy_option (options, copy, GRL_OPERATION_OPTION_FLAGS);
> +
> + return copy;
> +}
> +
> +/**
> + * grl_operation_options_key_is_set:
> + * @options: a #GrlOperationOptions instance
> + * @key: an operation option key
> + *
> + * This is an internal method that shouldn't be used outside of Grilo.
> + *
> + * Returns: whether @key is set in @options.
> + *
> + */
> +gboolean
> +grl_operation_options_key_is_set (GrlOperationOptions *options,
> + const gchar *key)
> +{
> + return g_hash_table_lookup_extended (options->priv->data, key, NULL, NULL);
> +}
> +
> +/**
> + * grl_operation_options_set_skip:
> + * @options: a #GrlOperationOptions instance
> + * @skip: number if elements to skip in an operation
Should be "number of elements"
> + *
> + * Set the skip option for an operation. Will only succeed if @skip obey to the
> + * inherent capabilities of @options.
Should be "If @skip obeys"
> + *
> + * Returns: %TRUE if @skip could be set, %FALSE otherwise.
> + */
> +gboolean
> +grl_operation_options_set_skip (GrlOperationOptions *options,
> + guint skip)
> +{
[...]
I didn't find any issue in the rest of the patch, so I'm skipping it for
readability :)
--
Simon Pena <spena igalia com>
Igalia - Free Software Engineering
Attachment:
signature.asc
Description: OpenPGP digital signature