Code review comment for ~alfonsosanchezbeato/network-manager:backport-bionic

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

« Back to merge proposal