Created attachment 151764 [details] [review]
http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/salut-wizard-607436

 src/Makefile.am                         |    1 +
 src/empathy-account-assistant.c         |    3 +
 src/empathy-auto-salut-account-helper.c |  233 +++++++++++++++++++++++++++++++
 src/empathy-auto-salut-account-helper.h |   35 +++++
 src/empathy.c                           |  184 +------------------------
 tests/interactive/Makefile.am           |    1 +
 6 files changed, 274 insertions(+), 183 deletions(-)

Review of attachment 151764 [details] [review]:

Looks good, some minor comments:

::: src/Makefile.am
@@ +48,3 @@
 	empathy-status-icon.c empathy-status-icon.h			\
 	empathy-sidebar.c empathy-sidebar.h				\
+	empathy-auto-salut-account-helper.c empathy-auto-salut-account-helper.h \

It's probably better to keep this list sorted alphabetically

::: src/empathy-auto-salut-account-helper.c
@@ +210,3 @@
+
+  tp_account_manager_prepare_async (manager, NULL,
+      create_salut_account_am_ready_cb, g_object_ref (managers));

Why do you _ref() managers here? You already duped the object in create_salut_account_if_needed(), so that reference should be already valid here.

::: tests/interactive/Makefile.am
@@ +35,3 @@
 	$(top_builddir)/src/empathy-import-widget.o	\
 	$(top_builddir)/src/empathy-import-pidgin.o	\
+	$(top_builddir)/src/empathy-auto-salut-account-helper.o	\

Also, better to keep this sorted.

(In reply to comment #2)
> ::: src/Makefile.am
> @@ +48,3 @@
>      empathy-status-icon.c empathy-status-icon.h            \
>      empathy-sidebar.c empathy-sidebar.h                \
> +    empathy-auto-salut-account-helper.c empathy-auto-salut-account-helper.h \
> 
> It's probably better to keep this list sorted alphabetically

Indeed. Done.

> ::: src/empathy-auto-salut-account-helper.c
> @@ +210,3 @@
> +
> +  tp_account_manager_prepare_async (manager, NULL,
> +      create_salut_account_am_ready_cb, g_object_ref (managers));
> 
> Why do you _ref() managers here? You already duped the object in
> create_salut_account_if_needed(), so that reference should be already valid
> here.

You're right. It did make sense when we were using the reference from the caller but not anymore. Fixed; nice catch!

> ::: tests/interactive/Makefile.am
> @@ +35,3 @@
>      $(top_builddir)/src/empathy-import-widget.o    \
>      $(top_builddir)/src/empathy-import-pidgin.o    \
> +    $(top_builddir)/src/empathy-auto-salut-account-helper.o    \
> 
> Also, better to keep this sorted.

Done.

I squashed the changes.

Created attachment 151772 [details] [review]
New patch

 src/Makefile.am                         |    1 +
 src/empathy-account-assistant.c         |    3 +
 src/empathy-auto-salut-account-helper.c |  233 +++++++++++++++++++++++++++++++
 src/empathy-auto-salut-account-helper.h |   35 +++++
 src/empathy.c                           |  184 +------------------------
 tests/interactive/Makefile.am           |    3 +-
 6 files changed, 275 insertions(+), 184 deletions(-)

Review of attachment 151772 [details] [review]:

Looks good to me.

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.