search

LNP-WG / lnp-node

Lightning network protocol daemon (suitable for generalized Lightning Network)
MIT License
145 stars 40 forks source link

Backport v0.9 various fixes to v0.8 #72

Closed dr-orlovsky closed 1 year ago

dr-orlovsky commented 1 year ago

As requested by @zoedberg in https://github.com/LNP-WG/lnp-node/pull/65

dr-orlovsky commented 1 year ago

@zoedberg can you pls accept invitation to this org such that I can assing a review to you?

zoedberg commented 1 year ago

sure, just accepted. @dr-orlovsky

dr-orlovsky commented 1 year ago

Looking at all these changes I have a serious doubt whence we need to do this "backporting". Most of them for sure are API-breaking etc. It will take a lot of time to review. Why to invest time into that if we have v0.9 and those needing this just need to adopt a new version?

Anyway wait for your opinion and review @zoedberg. I would not spend my time into sorting out what of this is a breaking change...

zoedberg commented 1 year ago

For us breaking changes at the API level are not an issue because we’re still in an early phase of lnp-node integration. But your observation is correct, it may be a problem for devs that already started integrating that (for example I think diba is already integrating lnp-node, right @cryptoquick?), so we probably should not backport everything but only non breaking changes (if any)

dr-orlovsky commented 1 year ago

I think that overall it should be nice to stick to the semver requirements than to rely on an assumption that nobody uses this. So is there a reason to backport (potentially breaking) new features from v0.9 to v0.8 if we already have v0.9 released?

Tagging @nicbus as one of the main proponents of semver which I know :D

nicbus commented 1 year ago

I think we all agree that sticking to semver and not having braking changes in a minor/patch release is best. therefore, as @zoedberg was also saying, what's desirable is to have only non-breaking changes backported into v0.8, but I also see your point about this requiring some time to review each change to assess if it has to be excluded

in the end, I think there are no problems in leaving these fixes out of v0.8 for the above reasons but, before we choose to do so, I'd wait for a go from @cryptoquick, as diba might need some of these

cryptoquick commented 1 year ago

We're fully updated to 0.9 here at DIBA, so I don't have a horse in this race.

zoedberg commented 1 year ago

ok so I guess we can close this PR without merging it and release a new lnp-node with just clap update @dr-orlovsky