Hi Wolfgang, Thanks for review !
>>I'd like to have more info on patch 0007-add-dummy-..., probably best >>to just write a more descriptive commit message there. yes, indeed. It's to avoid to launch ifupdown proxmox scripts(bridgevlanport,mtu), without remove them, as they are still provide by pve-manager package. I'll add description. >>Note that we started using git submodules with mirrors of upstream git >>repos for several packages by now, rather than having a `make download` >>target producing a tarball. We could do that here as well. I think it's >>more convenient, too. didn't known. I'll try to do this way. >>As for the patch workflow, I see there are a bunch which are going >>upstream and a bunch of pve-specific ones. Would be good to separate >>them somehow. (I haven't been very consistent in that regard, though. >>eg. in qemu we have a pve/ and an extra/ direcory, while in lxc we have >>the pve patches directly in the patches dir and a fixes/ subdir). >>One or the other might be nice to use. That way it's also more >>convenient to (automatically) update patch files if you have your custom >>changes as a branch in a git-clone. ok, I'll split both with pve/ and extra/ >>PS: You don't need to use a cover letter for single-patch mails, >>especially if it only copies the commit message, as it's already in the >>patch itself anyway ;-) Ok, I was not sure about this. Maybe add a note to https://pve.proxmox.com/wiki/Developer_Documentation ;) ----- Mail original ----- De: "Wolfgang Bumiller" <w.bumil...@proxmox.com> À: "Alexandre Derumier" <aderum...@odiso.com> Cc: "pve-devel" <pve-devel@pve.proxmox.com> Envoyé: Jeudi 17 Mai 2018 10:16:02 Objet: Re: [pve-devel] [PATCH 1/1] initial ifupdown2 package On Wed, May 16, 2018 at 12:01:40PM +0200, Alexandre Derumier wrote: > --- > Makefile | 42 +++++ > debian/changelog | 174 +++++++++++++++++++++ > debian/compat | 1 + > debian/control | 31 ++++ > debian/copyright | 28 ++++ > debian/ifupdown2.postinst | 86 ++++++++++ > ...0001-start-networking-add-usr-bin-in-PATH.patch | 28 ++++ > ...ns-scripts-fix-ENV-for-interfaces-options.patch | 29 ++++ > debian/patches/0003-config-tuning.patch | 52 ++++++ > .../0004-manual-interfaces-set-link-up.patch | 58 +++++++ > ...e-tap-veth-fwpr-interfaces-from-bridge-on.patch | 27 ++++ > ...6-netlink-IFLA_BRPORT_ARP_SUPPRESS-use-32.patch | 29 ++++ > ...0007-add-dummy-mtu-bridgevlanport-modules.patch | 69 ++++++++ > .../patches/0008-add-vxlan-physdev-support.patch | 159 +++++++++++++++++++ > debian/patches/series | 8 + > debian/rules | 21 +++ > 16 files changed, 842 insertions(+) > create mode 100644 Makefile > create mode 100644 debian/changelog > create mode 100644 debian/compat > create mode 100644 debian/control > create mode 100644 debian/copyright > create mode 100644 debian/ifupdown2.postinst > create mode 100644 > debian/patches/0001-start-networking-add-usr-bin-in-PATH.patch > create mode 100644 > debian/patches/0002-addons-scripts-fix-ENV-for-interfaces-options.patch > create mode 100644 debian/patches/0003-config-tuning.patch > create mode 100644 debian/patches/0004-manual-interfaces-set-link-up.patch > create mode 100644 > debian/patches/0005-don-t-remove-tap-veth-fwpr-interfaces-from-bridge-on.patch > > create mode 100644 > debian/patches/0006-netlink-IFLA_BRPORT_ARP_SUPPRESS-use-32.patch > create mode 100644 > debian/patches/0007-add-dummy-mtu-bridgevlanport-modules.patch > create mode 100644 debian/patches/0008-add-vxlan-physdev-support.patch > create mode 100644 debian/patches/series > create mode 100755 debian/rules > (...) Looks interesting, seems like you spent quite some time with this already. If we add this, a couple of things: I'd like to have more info on patch 0007-add-dummy-..., probably best to just write a more descriptive commit message there. Note that we started using git submodules with mirrors of upstream git repos for several packages by now, rather than having a `make download` target producing a tarball. We could do that here as well. I think it's more convenient, too. As for the patch workflow, I see there are a bunch which are going upstream and a bunch of pve-specific ones. Would be good to separate them somehow. (I haven't been very consistent in that regard, though. eg. in qemu we have a pve/ and an extra/ direcory, while in lxc we have the pve patches directly in the patches dir and a fixes/ subdir). One or the other might be nice to use. That way it's also more convenient to (automatically) update patch files if you have your custom changes as a branch in a git-clone. PS: You don't need to use a cover letter for single-patch mails, especially if it only copies the commit message, as it's already in the patch itself anyway ;-) _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
