Merge ~alfonsosanchezbeato/network-manager:add-wifi-ap-support into network-manager:snap-1.10

Proposed by Alfonso Sanchez-Beato
Status: Merged
Approved by: Tony Espy
Approved revision: 7e79ba7d7a32bb4747cddd37a04ebc7eeaac7b6d
Merged at revision: 4eaf370bc6c1944c3d40bfbe0d074cb0ff121044
Proposed branch: ~alfonsosanchezbeato/network-manager:add-wifi-ap-support
Merge into: network-manager:snap-1.10
Diff against target: 249 lines (+189/-1)
6 files modified
debian/patches/series (+1/-0)
debian/patches/set-ld-library-path-for-iptables.patch (+34/-0)
snap-common/bin/networkmanager (+5/-0)
snap-common/usr/share/doc/dnsmasq/copyright (+21/-0)
snap-patch/dnsmasq.patch (+102/-0)
snap/snapcraft.yaml (+26/-1)
Reviewer Review Type Date Requested Status
System Enablement Bot continuous-integration Approve
Sebastien Bacher Approve
Konrad Zapałowicz (community) Approve
Review via email: mp+369325@code.staging.launchpad.net

Commit message

Add missing bits to enable sharing connections and be able to access points. Fixes LP: #1832494.

Description of the change

Add missing bits to enable sharing connections and be able to access points. Fixes LP: #1832494.

To post a comment you must log in.
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks for the work, I'm not maintaining that snap/having real knowledge about it but since we want to consolidate the deb&snap packaging at some point I'm going to comment from that angle

Please don't add undocumented patches, set-ld-library-path-for-iptables.patch should have some header with a description on why the change is needed and references to upstream/downstream bugs as appropriate

It would be also good to describe if the patch would be acceptable for the deb and see if that's something can could be upstreamed in some way

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

@Sebastien thanks for the review, I will a description to the NM patch.

Re: deb, no, I do not think this patch should be used by the deb package, as what it does is making sure that iptables is able to locate the right libraries coming in the snap, which is not needed for the deb.

Revision history for this message
Konrad Zapałowicz (kzapalowicz) :
review: Approve
Revision history for this message
System Enablement Bot (system-enablement-ci-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Sebastien Bacher (seb128) wrote :

Looks fine to me but I know little about the snap so it should probably get another review from someone who does understand it better and knows how to do the testing

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

@Sebastien, well, that was already done by Konrad :)

Note also that automatic tests are passing: https://jenkins.canonical.com/system-enablement/job/snappy-hwe-snaps-snap-test/2205/console

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