Created attachment 204776 [details] [review]
vapi/upstream: update gdk, remove gtk bindings

Review of attachment 204776 [details] [review]:

1. I'll most probably need the gtk from git master for props so don't remove that just yet.
2. That mini-graph changes going under the title 'update gdk, remove gtk bindings' seems very wrong to me but since I'm sure we disagree strongly on that, I'd just suggest you point out that change in the commit log.
3. No objection to including gdk bindings.

(In reply to comment #2)
> Review of attachment 204776 [details] [review]:
> 
> 1. I'll most probably need the gtk from git master for props so don't remove
> that just yet.

for props?

> 2. That mini-graph changes going under the title 'update gdk, remove gtk
> bindings' seems very wrong to me but since I'm sure we disagree strongly on
> that, I'd just suggest you point out that change in the commit log.

it's related, since some of the api is now deprecated. In fact that's the only justification for updating the binding here.

(In reply to comment #3)
> (In reply to comment #2)
> > Review of attachment 204776 [details] [review] [details]:
> > 
> > 1. I'll most probably need the gtk from git master for props so don't remove
> > that just yet.
> 
> for props?

  Sorry, this I meant: https://github.com/gnome-design-team/gnome-mockups/raw/master/boxes/boxes-install5.5.png

> > 2. That mini-graph changes going under the title 'update gdk, remove gtk
> > bindings' seems very wrong to me but since I'm sure we disagree strongly on
> > that, I'd just suggest you point out that change in the commit log.
> 
> it's related, since some of the api is now deprecated. In fact that's the only
> justification for updating the binding here.

As I said, just mention that change in the log and we are good. :)

(In reply to comment #4)
> As I said, just mention that change in the log and we are good. :)

my bad, I thought I did when I --fixup. ok, will commit with update message.

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Review of attachment 204776 [details] [review] [details] [details]:
> > > 
> > > 1. I'll most probably need the gtk from git master for props so don't remove
> > > that just yet.
> > 
> > for props?
> 
>   Sorry, this I meant:
> https://github.com/gnome-design-team/gnome-mockups/raw/master/boxes/boxes-install5.5.png

Oh and the other of your latest changes require new gtk+ bindings too: https://bugzilla.gnome.org/review?bug=667435&attachment=204774.

app.vala:13.12-13.32: error: The type name `Gtk.ApplicationWindow' could not be found
    public Gtk.ApplicationWindow window;

Commit 7e70184d38a removed vapi/upstream/glib-2.0.vapi. This breaks compilation:

make[2] : on entre dans le répertoire « /home/teuf/jhbuild-boxes/sources/gnome-boxes/src »
  VALAC  gnome_boxes_vala.stamp
wizard.vala:279.26-279.36: error: The name `format_size' does not exist in the context of `Boxes.Wizard.review'
            var memory = format_size (resources.ram, FormatSizeFlags.IEC_UNITS);
                         ^^^^^^^^^^^
wizard.vala:279.17-279.79: error: var declaration not allowed with non-typed initializer
            var memory = format_size (resources.ram, FormatSizeFlags.IEC_UNITS);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
wizard.vala:289.48-289.53: error: The name `memory' does not exist in the context of `Boxes.Wizard.review'
            summary.add_property (_("Memory"), memory);
                                               ^^^^^^
wizard.vala:290.13-290.18: error: The name `memory' does not exist in the context of `Boxes.Wizard.review'
            memory = format_size (resources.storage, FormatSizeFlags.IEC_UNITS);
            ^^^^^^
wizard.vala:291.70-291.75: error: The name `memory' does not exist in the context of `Boxes.Wizard.review'
            summary.add_property (_("Disk"),  _("%s maximum".printf (memory)));
                                                                     ^^^^^^
Compilation failed: 5 error(s), 0 warning(s)


 git checkout 7e70184d38a785840aecc858c705994f866766df^ vapi/upstream/glib-2.0.vapi && make is successful

(In reply to comment #7)
> Commit 7e70184d38a removed vapi/upstream/glib-2.0.vapi. This breaks
> compilation:
> 
> make[2] : on entre dans le répertoire «
> /home/teuf/jhbuild-boxes/sources/gnome-boxes/src »
>   VALAC  gnome_boxes_vala.stamp
> wizard.vala:279.26-279.36: error: The name `format_size' does not exist in the
> context of `Boxes.Wizard.review'
>             var memory = format_size (resources.ram,
> FormatSizeFlags.IEC_UNITS);
>                        ^^^^^^^^^^^

Yikes, I missed the fact that Marc-Andre was silently (gtk+/gdk vapi changes mentioned in the commit summary but glib) removing glib vapi. :( This is why I don't like separate changes into the same commit/patch as its hard to then summarize all changes in the commit log and easy to miss important changes.

isn't FormatSizeFlags.IEC_UNITS in the last vala release? Can't we bump on vala 0.15 depedency instead?

(In reply to comment #9)
> isn't FormatSizeFlags.IEC_UNITS in the last vala release? Can't we bump on vala
> 0.15 depedency instead?

It is and I'm always good with bumping dep when it comes to projects following GNOME release cycle.

(In reply to comment #10)
> (In reply to comment #9)
> > isn't FormatSizeFlags.IEC_UNITS in the last vala release? Can't we bump on vala
> > 0.15 depedency instead?
> 
> It is and I'm always good with bumping dep when it comes to projects following
> GNOME release cycle.

Then again, I intend to add new API to glib this week (and therefore vapi update to vala git) and unless I fail, I'll be re-adding the glib vapi for the same reasons it was added originally.