Re: [PATCH 1/4] libnm-core: add wake-on-wlan configuration items

On 18.01.2017 15:17, Thomas Haller wrote:
On Tue, 2017-01-17 at 15:38 +0100, Simon Fels wrote:
 libnm-core/nm-core-enum-types.c  | 27 +++++++++++++
 libnm-core/nm-core-enum-types.h  |  2 +
 libnm-core/nm-setting-wireless.c | 85
 libnm-core/nm-setting-wireless.h | 47 ++++++++++++++++++++++
 libnm/libnm.ver                  |  3 ++
 libnm/nm-setting-docs.xml        |  2 +
 6 files changed, 166 insertions(+)

Hi Simon,

Thanks for the patches.

the patchset is for 1.2 branch (but doesn't apply there because of 
libnm-core/nm-core-enum-types.h, which is a generated file).

We always apply patches first on master, and backport from there to old
branches. Never the other way around.

This is especially the case when adding new public API. You cannot ever
add new API to a stable branch that is not already in master.

Would you be willing to rebase it to master?

Sure, stated that already in another mail I've sent directly after
sending out the patches and noticing that I've sent the wrong ones.

Just saw, that mail only went back to myself and not to the mailing
list... sorry for that :-)

diff --git a/libnm-core/nm-core-enum-types.c b/libnm-core/nm-core-
index 83f5e88..58581bb 100644
--- a/libnm-core/nm-core-enum-types.c
+++ b/libnm-core/nm-core-enum-types.c

these files are generated, they shall not be part of the patchset.

+ * nm_setting_wireless_get_wake_on_wlan:
+ * @setting: the #NMSettingWireless
+ *
+ * Returns the Wake-on-WLAN options enabled for the connection
+ *
+ * Returns: the Wake-on-WLAN options
+ *
+ * Since: 1.2
+ */

you cannot add new API after 1.2.0 is released, and claim it is 1.2 API
(it is not).

What you can do, is backport API from newer versions to minor stable
releases, like for example like when we backported
nm_setting_wired_get_wake_on_lan from 1.2, to 1.0.6:

this also needs to add the backported symbols to all newer versions to
allow the upgrade path:

But doing so is a major pain, so usually it's just simpler not to
extend already released stable API.

Personally, I think it is also a mistake to add API downstream first
(or downstream only). Adding API can only be done first in upstream
master, then backported to the corresponding stable-branch, and then
rebase the downstream package to a new stable-release with the new API.
Doing it any other way, prevents a user to use an upstream version of
libnm, or upgrade to a newer version.

For our use case its good enough to do it this way as we have a version
of network-manager we use in an environment where only NetworkManager
itself uses libnm and nothing else and we're stuck for now to 1.2.2 and
as we're not upstream we can't easily say we release a new minor version.

However you're right and I will correct these things with the next
review round to get everything correct for inclusion in master.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]