Merge ~alfonsosanchezbeato/network-manager:backport-bionic into network-manager:bionic

Proposed by Alfonso Sanchez-Beato
Status: Approved
Approved by: Tony Espy
Approved revision: da2569073d4f86a2334488b4c843cd25f623363b
Proposed branch: ~alfonsosanchezbeato/network-manager:backport-bionic
Merge into: network-manager:bionic
Diff against target: 932 lines (+903/-2)
3 files modified
debian/changelog (+7/-2)
debian/patches/Support-for-WoWLAN.patch (+895/-0)
debian/patches/series (+1/-0)
Reviewer Review Type Date Requested Status
System Enablement Bot continuous-integration Needs Fixing
Tony Espy Approve
Review via email: mp+349465@code.staging.launchpad.net

Description of the change

debian/patches/Support-for-WoWLAN.patch: backport WoWLAN support

Backport WoWLAN support from NM upstream 1.12/1.14 (LP: #1781597)

To post a comment you must log in.
Revision history for this message
Tony Espy (awe) wrote :

I've compared this to upstream, and the only thing that seems odd is that the last four commits you list are not part of any 1.12.x branch, but only exist in master:

ac1302793 platform: add methods to retrieve current WoWLAN state
c6e40215e devices: restore past WoWLAN when disconnecting wifi
a3289400d wifi: ensure wake-on-wlan restore only acts once
2c3a14fed platform/wifi: drop *_get_wowlan()

Looking closer, the last two are a bug fix and cleanup of unused code submitted by upstream developers, whereas the first two are commits from you. Why didn't these land in 1.12?

Given these differences, your MP description and debian/changelog aren't accurate, as in addition to backporting from 1.12, you've also pulled from master too. I'm not as concerned about the MP description (although it would've been helpful in advance), but you should mention this in the changelog, and maybe in the patch too.

Can you also please update the dates in your debian/changelog and .patch file to be current?

Also have you pushed versions to a PPA anywhere that can be tested, or do you want to just wait and test the version that gets pushed to bionic-proposed?

review: Needs Fixing
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

Hm, not sure why they did not include those commits, I thought they had. I think they just took the minimum changes to get working WoWLAN in. Of those 4 commits, 3 are for restoring previous WoWLAN settings if, say, we had some WoWLAN setting set with "iw", then used NM to set something different. The patches make sure we put the original WoWLAN settings when we unset WoWLAN from NM instead of just disabling it. As you see, is quite a corner case and we can live without those patches. The 4th patch is just some clean-up.

No, I do not have a PPA, but I tested this building from sources in the 1.10 branch. We can test on bionic-proposed when the package is ready.

Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

@Tony, MP refreshed after updating changelog/comments.

Revision history for this message
Tony Espy (awe) wrote :

LGTM

review: Approve
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)

There was an error fetching revisions from git servers. Please try again in a few minutes. If the problem persists, contact Launchpad support.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches