Created attachment 224883 [details] [review]
Add version API

Comment on attachment 224883 [details] [review]
Add version API

Is this based on some other library's API? It seems like having three separate functions is annoying, since you're generally going to need to check at least two of them... you could just have:

    void soup_get_version (int *major, int *minor, int *micro);

and/or

    gboolean soup_check_version (int major, int minor, int micro);

>+m4_define([soup_major_version], [2])
>+m4_define([soup_minor_version], [39])
>+m4_define([soup_micro_version], [92])
>+
> AC_PREREQ(2.63)
> AC_INIT([libsoup],[2.39.92],[http://bugzilla.gnome.org/enter_bug.cgi?product=libsoup])

Is it possible to use the new variables inside the AC_INIT call?

>+/**
>+ * SECTION:soup-version
>+ * @short_description: Variables and functions to check the libsoup version
>+ **/

All the docs need Since: tags. Which then leads to the question of whether or not you were hoping to get this into 2.40 or not.

Also, please put the macro docs in the .c file rather than the .h file.

>+ * Returns the major version number of the libsoup library.
>+ * (e.g. in libsoup version 1.8.3 this is 1.)

There was never a libsoup 1.8.3... Use "2.40.0".

>+ * @major: major version (e.g. 1 for version 1.2.5)

Nor was there a libsoup 1.2.5. :)

(In reply to comment #2)

Thanks for the review!

> (From update of attachment 224883 [details] [review])
> Is this based on some other library's API? It seems like having three separate
> functions is annoying, since you're generally going to need to check at least
> two of them... you could just have:
> 
>     void soup_get_version (int *major, int *minor, int *micro);

I've left the individual functions, because a method like this is harder use via introspection in other languages. It also fits with the style of both GTK+ and WebKitGTK+.

> and/or
> 
>     gboolean soup_check_version (int major, int minor, int micro);

I have added this one though

> >+m4_define([soup_major_version], [2])
> >+m4_define([soup_minor_version], [39])
> >+m4_define([soup_micro_version], [92])
> >+
> > AC_PREREQ(2.63)
> > AC_INIT([libsoup],[2.39.92],[http://bugzilla.gnome.org/enter_bug.cgi?product=libsoup])
> 
> Is it possible to use the new variables inside the AC_INIT call?

Yes, this was a silly oversight on my part. :)

> 
> >+/**
> >+ * SECTION:soup-version
> >+ * @short_description: Variables and functions to check the libsoup version
> >+ **/
> 
> All the docs need Since: tags. Which then leads to the question of whether or
> not you were hoping to get this into 2.40 or not.

I've added Since: 2.40tags to everything.

> Also, please put the macro docs in the .c file rather than the .h file.

Done.
 
> >+ * Returns the major version number of the libsoup library.
> >+ * (e.g. in libsoup version 1.8.3 this is 1.)
> 
> There was never a libsoup 1.8.3... Use "2.40.0".
> 
> >+ * @major: major version (e.g. 1 for version 1.2.5)
> 
> Nor was there a libsoup 1.2.5. :)

Fixed all these.

Created attachment 224945 [details] [review]
Updated according to review comments

committed, with various numbers updated to reflect master. sorry for
the delay

Thanks for the commit!