[Merge] lp:~khurshid-alam/indicator-sound/fix-build-impish into lp:indicator-sound

Dmitry Shachnev mp+409379 at code.launchpad.net
Wed Sep 29 16:56:59 UTC 2021


In general this looks good to me, thanks!

Diff comments:

> === modified file 'debian/control'
> --- debian/control	2020-06-04 12:59:23 +0000
> +++ debian/control	2021-09-29 16:33:39 +0000
> @@ -32,7 +32,7 @@
>                 libxml2-dev,
>                 pulseaudio,
>                 python3-dbusmock,
> -               qt5-default,
> +               libgmock-dev,

Please keep this list sorted. libgmock-dev should better go after libglib2.0-dev.

>                 qtbase5-dev,
>                 qtbase5-dev-tools,
>                 qtdeclarative5-dev,
> 
> === modified file 'debian/rules'
> --- debian/rules	2020-06-04 12:59:23 +0000
> +++ debian/rules	2021-09-29 16:33:39 +0000
> @@ -17,5 +17,6 @@
>  	install -m 644 debian/indicator-sound-crashdb.conf debian/indicator-sound/etc/apport/crashdb.conf.d/
>  	dh_install --fail-missing
>  
> +# For live test logs:
>  override_dh_auto_test:
> -	-timeout 10m dh_auto_test
> +	ARGS=-V dh_auto_test

Making the tests not ignored is a good thing, thanks!

But are you sure timeout 10m is no longer needed?

> 
> === modified file 'src/media-player-list-greeter.vala'
> --- src/media-player-list-greeter.vala	2014-04-01 23:20:56 +0000
> +++ src/media-player-list-greeter.vala	2021-09-29 16:33:39 +0000
> @@ -45,8 +45,13 @@
>  			this.proxy.entry_selected.connect(active_user_changed);
>  			this.proxy.get_active_entry.begin ((obj, res) => {
>  				try {
> -					var value = (obj as UnityGreeterList).get_active_entry.end(res);
> -					active_user_changed(value);
> +					var list = (obj as UnityGreeterList);
> +
> +					if (list != null) {
> +						

Please remove blank line after {.

> +						var value = list.get_active_entry.end(res);
> +						active_user_changed(value);
> +					}
>  				} catch (Error e) {
>  					warning("Unable to get active entry: %s", e.message);
>  				}


-- 
https://code.launchpad.net/~khurshid-alam/indicator-sound/fix-build-impish/+merge/409379
Your team Indicator Applet Developers is requested to review the proposed merge of lp:~khurshid-alam/indicator-sound/fix-build-impish into lp:indicator-sound.




More information about the Ubuntu-reviews mailing list