Created attachment 250282 [details] [review]
drop GtkStock

Could you give some background for the change, like why it is needed (of course, I might miss it on _some_ mailing list or so, but it's still good to know the background for the change, before doing it out of nowhere.

Review of attachment 250282 [details] [review]:

::: modules/trust-prompt/certificate-viewer.c
@@ +573,3 @@
 		title, parent,
 		GTK_DIALOG_DESTROY_WITH_PARENT | GTK_DIALOG_MODAL,
+		_("_Close"), GTK_RESPONSE_CLOSE,

This adds a new translatable string into evolution-data-server, while it wasn't needed before.

GtkStock is deprecated in GTK+ 3.10 (now in master).

See:
https://mail.gnome.org/archives/gtk-devel-list/2013-July/msg00000.html
https://docs.google.com/document/d/1KCVPoYQBqMbDP11tHPpjW6uaEHrvLUmcDPqKAppCY8o/pub
https://docs.google.com/spreadsheet/pub?key=0AsPAM3pPwxagdGF4THNMMUpjUW5xMXZfdUNzMXhEa2c&output=html

(In reply to comment #3)
> Review of attachment 250282 [details] [review]:
> 
> ::: modules/trust-prompt/certificate-viewer.c
> @@ +573,3 @@
>          title, parent,
>          GTK_DIALOG_DESTROY_WITH_PARENT | GTK_DIALOG_MODAL,
> +        _("_Close"), GTK_RESPONSE_CLOSE,
> 
> This adds a new translatable string into evolution-data-server, while it wasn't
> needed before.

This adds a new translatable strings into each apps gnome.

(In reply to comment #5)
> This adds a new translatable strings into each apps gnome.

Yeah, and it's the reason why I feel sorry for their decision, because instead of having the most common strings in one base library, it'll be distributed to each application which uses the library. See how much work it adds to translators.

Review of attachment 250282 [details] [review]:

Disregard the previous review comment about translation, and just fix the icon name. Then feel free to commit to master. Thanks.

::: modules/trust-prompt/trust-prompt-gtk.c
@@ +187,3 @@
 	gtk_container_add (GTK_CONTAINER (widget), GTK_WIDGET (grid));
 
+	widget = gtk_image_new_from_icon_name ("dialog-warning-symbolic", GTK_ICON_SIZE_DIALOG);

change the icon name to "dialog-warning", please. It's the name listed at
https://docs.google.com/spreadsheet/pub?key=0AsPAM3pPwxagdGF4THNMMUpjUW5xMXZfdUNzMXhEa2c&output=html

(In reply to comment #7)
> Review of attachment 250282 [details] [review]:
> 
> Disregard the previous review comment about translation, and just fix the icon
> name. Then feel free to commit to master. Thanks.
> 
> ::: modules/trust-prompt/trust-prompt-gtk.c
> @@ +187,3 @@
>      gtk_container_add (GTK_CONTAINER (widget), GTK_WIDGET (grid));
> 
> +    widget = gtk_image_new_from_icon_name ("dialog-warning-symbolic",
> GTK_ICON_SIZE_DIALOG);
> 
> change the icon name to "dialog-warning", please. It's the name listed at
> https://docs.google.com/spreadsheet/pub?key=0AsPAM3pPwxagdGF4THNMMUpjUW5xMXZfdUNzMXhEa2c&output=html

not...
the postfix of each the symbolic icons is '-symbolic'.

see:
https://git.gnome.org/browse/gnome-icon-theme-symbolic/tree/gnome/scalable/status
and https://git.gnome.org/browse/gnome-icon-theme-symbolic/tree/gnome/scalable/status/dialog-warning-symbolic.svg

Well, the table I quoted says something else. The gtk_image_new_from_icon_name() defines both the name, and the requested size. It's the GtkIconTheme work to pick the right icon, and I do not like to see a hard-coding of something what is not in the migration documentation.

Furthermore, using the icon from gnome-icon-theme-symbolic would require to add dependency on it in evolution-data-server, which feels wrong as well. Please use _the standard_ icon name.

(In reply to comment #9)
> Well, the table I quoted says something else. The
> gtk_image_new_from_icon_name() defines both the name, and the requested size.
> It's the GtkIconTheme work to pick the right icon, and I do not like to see a
> hard-coding of something what is not in the migration documentation.
> 
> Furthermore, using the icon from gnome-icon-theme-symbolic would require to add
> dependency on it in evolution-data-server, which feels wrong as well. Please
> use _the standard_ icon name.

"dialog-warning" is from gnome-icon-theme,
"dialog-warning-symbolic" is from gnome-icon-theme-symbolic.

in the new design of gnome, need "dialog-warning-symbolic".

see:
https://bugzilla.gnome.org/show_bug.cgi?id=704297
and the commit:
https://git.gnome.org/browse/gedit/commit/?id=ee6b232c37d469b3e857c8d2093d88652475c7c7

(In reply to comment #10)
> "dialog-warning" is from gnome-icon-theme,
> "dialog-warning-symbolic" is from gnome-icon-theme-symbolic.

yes, I understood that, which might be seen by the "added dependency" note in comment #9.

> https://bugzilla.gnome.org/show_bug.cgi?id=704297

I do not see anything wrong on the icon being used there. If they want to use a different icon, then they should do so in gnome-icon-theme transparently, not forcing everyone changing to their needs, and requesting two packages instead of one. This way of work and decision making seems rather suboptimal to me. I know it's not your fault, I only express my feelings.

In any case, your patch is incomplete, with respect of dependency definition, but I'd still prefer to use the standard icon name, not the symbolic one.

Created attachment 250564 [details] [review]
drop GtkStock

Review of attachment 250564 [details] [review]:

Thanks. Please commit to master.

Review of attachment 250564 [details] [review]:

pushed as 230c2e4cb4aa8ce086eaf6b0b8011d7cf4aa69f0 - drop GtkStock