The Sound Menu

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

fix-build-impish
Merge into trunk.16.10

Proposed by Khurshid Alam on 2021-09-29

Status:	Merged
Merged at revision:	565
Proposed branch:	lp://staging/~khurshid-alam/indicator-sound/fix-build-impish
Merge into:	lp://staging/indicator-sound
Diff against target:	1043 lines (+170/-104) 24 files modified debian/changelog (+36/-0) debian/control (+1/-1) debian/rules (+2/-1) src/CMakeLists.txt (+0/-1) src/accounts-service-access.vala (+3/-3) src/freedesktop-interfaces.vala (+2/-2) src/greeter-broadcast.vala (+5/-5) src/media-player-list-greeter.vala (+7/-3) src/media-player-list-mpris.vala (+1/-1) src/media-player-mpris.vala (+1/-1) src/media-player-user.vala (+15/-3) src/mpris2-interfaces.vala (+8/-8) src/notification.vala (+1/-1) src/options-gsettings.vala (+5/-5) src/sound-menu.vala (+2/-0) src/volume-control-pulse.vala (+6/-0) src/volume-control.vala (+1/-1) src/volume-warning.vala (+4/-3) tests/CMakeLists.txt (+1/-1) tests/indicator-test.cc (+3/-3) tests/integration/indicator-sound-test-base.cpp (+7/-2) tests/integration/test-indicator.cpp (+49/-49) tests/notifications-test.cc (+9/-9) tests/volume-control-test.cc (+1/-1)
To merge this branch:	bzr merge lp://staging/~khurshid-alam/indicator-sound/fix-build-impish
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Gunnar Hjalmarsson		2021-09-29	Approve on 2021-09-29
Review via email: mp+409379@code.staging.launchpad.net

Commit message

* Fix build against vala > 48 & vala >= 50

* Add symbolic icons to the tests

* Avoid possible null value in Greeterlist and GreeterBroadcast object.

See https://github.com/AyatanaIndicators/ayatana-indicator-sound/commit/8df9168f5587cfd31e0bf6a4170a4c4f9784dbb7

* Fix build against GLib > 2.64: Replace HashTable with GenericSet

* Fix static member access

* Drop deprecated DBusProxy.create_for_bus

* Add libdbustest-1 include path

* Unit Tests: Unset environment variable and session bus in teardown

* debian/control: Remove qt5-default & add libgmock-dev in build depends. See https://bugs.launchpad.net/bugs/1921781

* Unit Tests: The desktop pulseaudio daemon also needs the stream-restore module these days

* Unit Tests: Disable notification tests that can not be fixed now. Most of them are phone related and depends on liburl-dispatcher library which was dropped from code since it is no longer in the universe repository. See https://bazaar.launchpad.net/~indicator-applet-developers/indicator-sound/trunk.16.10/revision/564

* Unit Tests: Disable indicator tests that can not be fixed now. These tests needs to be re-enabled later
   - tests/indicator-test.cc:PhoneMenu
   - tests/indicator-test.cc:DesktopMenu
   - tests/indicator-test.cc:BaseActions
   - tests/integration/test-indicator.cpp:All tests
   - tests/volume-control-test.cc:BasicObject

* debian/rules: Re enable live tests logs

Description of the change

Regarding tests, most only works on unity-8/phone environment and some are failing due to bugs libdbustest and gmock. As per my testing this doesn't hamper functionality on desktop, indicator-sound works as expected. These tests can be enabled for desktop later if someone able to fix those.

Revision history for this message

Dmitry Shachnev (mitya57) wrote on 2021-09-29:

In general this looks good to me, thanks!

Revision history for this message

Gunnar Hjalmarsson (gunnarhj) wrote on 2021-09-29:

I built it successfully on multiple archs:

https://launchpad.net/~gunnarhj/+archive/ubuntu/indicator-sound/+packages

So I'm ready to sponsor provided that you

* address Dmitry's questions/suggestions, and

* summarize the changes in a proper d/changelog entry.

review: Needs Fixing

lp://staging/~khurshid-alam/indicator-sound/fix-build-impish updated on 2021-09-29

584. By Khurshid Alam on 2021-09-29: Keep dependency order sorted. Remove empty lines

Revision history for this message

Khurshid Alam (khurshid-alam) wrote on 2021-09-29 (last edit on 2021-09-29):

@Gunner

Hi,

1. I addressed Dimitry's suggestions.

2. For debian/changelog, the way it works for bzr launchpad, I think, I don't change it. You merged and then released with d/changelog modified.

But I can still change from my end if it is required. Pleae let me know in comments.

Revision history for this message

Gunnar Hjalmarsson (gunnarhj) wrote on 2021-09-29:

Since it's multiple changes, I'd prefer if you include the d/changelog change in your merge request. (I think the practice differs between different teams, but in this case, as a sponsor, I ask you to do it.)

lp://staging/~khurshid-alam/indicator-sound/fix-build-impish updated on 2021-09-29