🏗 Integrate open PRs 🚧

[sbernard31]

There is a lot of open PR, this issue aims to centralize discussion about integrating this PR in master to put the project back on track.

Following @sbertin-telular, the first one should be : #423.

[rettichschnidi]

@sbernard31 Proposal:

• Now that PR CI: Build and test using GitHub Actions #489 got merged, all PRs should get rebased on current master in order to run this new check. See PR Resolve warning when building with -Wextra and -Wno-unused-parameter #471 for an example. This would also have the nice side-effect of knowing for which PRs the creator is still interested in/willing to invest time into.
• While being at it, asking the PR creator to squash the commits sounds worthwhile to me too (but maybe a bit too much to ask as for this project there seems to be no such requirement/recommendation for PRs as of today)

[sbernard31]

Now that PR #489 got merged, all PRs should get rebased on current master in order to run this new check. See PR #471 for an example. This would also have the nice side-effect of knowing for which PRs the creator is still interested in/willing to invest time into.

@rettichschnidi, It sounds a good idea, but should we integrate "our" PR first ? I mean PR written by you and your teammates and @sbertin-telular. The idea is to avoid that too many people resolve conflict at the same time and so by the way generate new one ? (I'm not sure I'm clear 🤔)

While being at it, asking the PR creator to squash the commits sounds worthwhile to me too (but maybe a bit too much to ask as for this project there seems to be no such requirement/recommendation for PRs as of today)

We can ask to politely squash commit and also say that if they can not we will do it (once PR is rebased without conflict, it's easy to squash.)

About recommendation, I will create an issue to discuss about creating/adding all "classic" community documentation.

[sbertin-telular]

I was going to suggest #427 after #423 is merged, but if we're going to do "ours" first that would be #430.

[sbernard31]

@sbertin-telular, @rettichschnidi, Do you want to review your PR each other before I integrate it in master ? or for those old PRs I can merge it without review ?

[sbertin-telular]

I think it would be good for them to be reviewed. I'm not sure if they've had much of a review before now.

[sbernard31]

#423 is integrated. Let's go for the next one.

[sbernard31]

#430 is integrated. Let's go for the next one.

[sbertin-telular]

#432 is next.

[sbernard31]

#432 is integrated. Let's go for the next one. (I'm a kind of github robot 🤖)

[sbertin-telular]

I think the next couple are #435 and #446. Maybe #461 after that if it is ready. I also have many changes I haven't created pull requests for yet.

[tuve]

I noticed i have to rebase #461 . There has also been an internal code review from some of the embedded developers in a different department for their patch set from an older version of wakaama. Not all is related to #461 but other parts could possible be reported as bugs I guess? The person who has done this as part of his assignment will not be available to report here as he is leaving for another assignment next week, but I will make sure we get it in to the code review, either by me or some colleague of mine.

[sbernard31]

#435 is integrated. Let's go for the next one.

[sbernard31]

#437 and #396 was closed because it should be fixed by #435.

[sbernard31]

#446 is integrated. Let's go for the next one.

[sbertin-telular]

Most of my remaining changes for LWM2M 1.1 started from the assumption that #431 would be merged first. I don't have PRs open for those changes yet. Do we want to skip #431 for now and I'll adjust my changes or get it merged? I do think it helps with understanding the structure of the code vs having everything under core.

[sbernard31]

I just look at #431, it is a code reorg/refactoring. Currently this PR is not in mergeable state (lot of conflicts).

As this is a lot of file moves, I suspect resolving conflicts will be pretty much the same effort than rewrite it. Anyway in all case I think maybe you should create a new PR (then cherry-picked david's commit or rewrite it on your own).

Maybe before to do that we should see if there is not PR (not specially yours) which could be easier to integrate before ?

WDYT ?

[sbertin-telular]

Probably best to get other open PRs merged first. That will make it easier for the contributors. I can create a new PR to do the reorg later.

@tuve are you close on being ready to merge #461, or should we work to get other PRs merged first?

[tuve]

I think @qleisan can finish up the comments otherwise I should be able to prioritize #461 later half of the upcoming week.

[qleisan]

@tuve I'm looking into it already. Found some issues and would like to convince myself that it works as intended.

[sbertin-telular]

#463, #483, and #485 look good to me.

[sbernard31]

#463, #483 are integrated in master. (it seems that #485 was already fixed).

[sbernard31]

5 opened PR left. We are on good track. 👍

[qleisan]

It is good with clarity on what issues that must be fixed together with block transfer #461 and what can be fixed after it has been merged

[qleisan]

@sbertin-telular @sbernard31 what is remaining to implement in order to have lwm2m 1.1 functionality once #461 and "your branch" is delivered? Can we make a plan for a 1.1 release (content and testing). An official release (with good quality) will send a strong signal to the community.

[sbernard31]

@qleisan, I suppose this is the roadmap item(6.) from #488.
Do not hesitate to create a dedicated issue and you can discuss what is desired for a next release, what is already implemented and so what is missing.

Once you agree on it. Either we keep the issue as roadmap or you can create wiki dedicated page(s).

Just as an example at Leshan side we have this :

• https://github.com/eclipse/leshan/wiki/Roadmap
• https://github.com/eclipse/leshan/wiki/LWM2M-Supported-features
• https://github.com/eclipse/leshan/wiki/LWM2M-1.1-supported-features

[sbernard31]

By the way as soon as #461 is merged, I understand that #431 should be the next one. (@sbertin-telular is on it if I'm not wrong)

[sbernard31]

#461 is integrated. @sbertin-telular #431 is still the next one in the plan ?

[sbertin-telular]

I just created #528 to replace #431.

[sbertin-telular]

It has been a long time since I've seen no open pull requests. Good to see it. I'll start working to create more with my LWM2M 1.1 changes.

[sbernard31]

Just to let you know that currently, I integrate PR as soon as I get 1 approval.
Feel free to review after integration and comment PR anyway.

If you think I should do differently, maybe waiting 1 day(or more) after approval to let a chance to get more review, let me know 🙏 !

[sbernard31]

Do we still need this issue or can I close it now ?
I guess we are back on track about PR integration and don't need it anymore ? 🤔

[qleisan]

I think it is ok to close this issue, the major PR:s have been coordinated and merged