Created attachment 232237 [details] [review]
Add default handler for ClutterSwipeAction::swipe

So code that still uses the deprecated ::swept keeps working

Review of attachment 232237 [details] [review]:

The patch does not make any sense. ::swipe does not have return value, and ::swept was introduced for that reason.

If the callback has a void return type, why are you returning a Boolean value?

(In reply to comment #2)
> Review of attachment 232237 [details] [review]:
> 
> The patch does not make any sense. ::swipe does not have return value, and
> ::swept was introduced for that reason.
> 
> If the callback has a void return type, why are you returning a Boolean value?

::swipe Is the new signal, and it does have a return value. ::swept is the old one, which stopped working when ::swipe was introduced.

tests/interactive/test-swipe-action.c wasn't updated (nor ran, apparently) when ::swipe was added.

(In reply to comment #3)
> (In reply to comment #2)
> > Review of attachment 232237 [details] [review] [details]:
> > 
> > The patch does not make any sense. ::swipe does not have return value, and
> > ::swept was introduced for that reason.
> > 
> > If the callback has a void return type, why are you returning a Boolean value?
> 
> ::swipe Is the new signal, and it does have a return value. ::swept is the old
> one, which stopped working when ::swipe was introduced.
> 
> tests/interactive/test-swipe-action.c wasn't updated (nor ran, apparently) when
> ::swipe was added.

I'm not contesting the validity of the claim that SwipeAction is broken: I'm contesting that the patch makes even remotely sense. sadly, I didn't have enough connectivity to do a proper review, so lemme do one properly.

Review of attachment 232237 [details] [review]:

::: clutter/clutter-swipe-action.c
@@ +171,3 @@
 
+/* XXX:2.0 remove */
+static void

if the return type is 'void' then...

@@ +177,3 @@
+       gpointer               data_)
+{
+    return TRUE;

... this cannot return TRUE.

also, the coding style is wrong: it should be two spaces, not four.

@@ +193,3 @@
 
+  /* XXX:2.0 remove */
+  klass->swipe = swipe;

it's probably better to give a name like 'clutter_swipe_action_real_swipe', to avoid names collisions (I doubt we'll get one, but one-word functions tend to raise that kind of stuff).

Created attachment 232644 [details] [review]
Add default handler for ClutterSwipeAction::swipe

So code that still uses the deprecated ::swept keeps working

(In reply to comment #5)
> Review of attachment 232237 [details] [review]:
> 
> ::: clutter/clutter-swipe-action.c
> @@ +171,3 @@
> 
> +/* XXX:2.0 remove */
> +static void
> 
> if the return type is 'void' then...
> 
> @@ +177,3 @@
> +       gpointer               data_)
> +{
> +    return TRUE;
> 
> ... this cannot return TRUE.
> 
> also, the coding style is wrong: it should be two spaces, not four.

Now I got what you meant, sorry about that :)

thanks; pushed to the master and clutter-1.14 branches.