[RFC] Drastic change of PR/review/merge process

[nordicjm]

It seems that a big change is needed for mcuboot in order to better work for everyone - contributors, users and reviewers. I am submitting this as an "RFC" for how the current process is lacking and can be improved. Starting with the issues as I see them today:

• PRs submitted are not assigned to anyone and unless manually added to the mcuboot repo as someone with write access, tags cannot be added nor are they assigned automatically.
• With the above issue, it becomes a game of chance if someone reviews a PR because either they've been added as a reviewer manually, been told about it by the author or stumbled into it whilst browsing the PR page.
• Issues have a similar problem in that unless you specifically tag someone in an issue, they can go unheard for long periods of time. For example, a question on TLV assignment from November 2022: Implementation-reserved TLVs and/or TLV magic values #1510 and a bug from the same month: espressif:esp32 fail to load app due to overwrite of iram_loader_seg #1518 both without any assignments or comments.
• Some issues/requests/PRs are posted with what seems to be little care for existing users of mcuboot, people expect to have a reliable bootloader and bootloader library for applications which includes bug fixes and having a breaking change that then means they can no longer use e.g. the bootloader libraries going forward is very much undesired.

So for addressing these issues, I am suggesting the following:

• PRs submitted should have reviewers added automatically, perhaps a zephyr system could be created whereby files specific to an OS are assigned to whomever is actively maintaining that port, but for common code or library code I would suggest that there should be at least 1 maintainer nominated as the lead for each actively supported OS and they're all added to these PRs so that they get a notification of the change and are invited to review it (maintainers should not need to stumble into PRs on chance of viewing the PR page). Tags can also be automatically added by a bot to show what areas of mcuboot have been altered which also allows for easy searching using labels.
• More checking of issues is needed, perhaps a better labeling system would help with this e.g. "bug", "enhancement", "query", etc. and then tags for specific ports if people are using them. How this can be done by normal users that are not part of the mcuboot group I do not know, maybe with a bot that monitors what is being posted and looks for key words. It would also be good to have priority, status and time-frames added to issues, e.g. is a bug urgent or low priority? Is a fix being worked on? Is the fix expected to arrive in the next mcuboot version or is it a year away?
• Unless there is a very good reason for breaking behaviour with the existing releases of mcuboot or libraries, I do not think breaking changes should be introduced or suggested. And for any that are they should not be merged unless at least 1 person from each supported OS has been invited to review and has left their comments (or a specified minimum period of time has passed and they have not left any comments) to avoid causing unexpected problems for OS maintainers and mcuboot users. For minimum times to allow for reviews, I suggest the times used by zephyr from https://docs.zephyrproject.org/latest/project/dev_env_and_tools.html#give-reviewers-time-to-review-before-code-merge for the TSC group of 1 week, as mcuboot might not be something those people are working on most of the time.

With the above changes, I think managing mcuboot becomes far easier, people won't be left waiting stuck with queries on why things are not working or with bug fixes, maintainers for each OS would then be actively invited to review PRs and know about changes that are going on and it makes it far easier when there is a process that is documented for how the project and additions are managed.

I welcome comments and suggestions on the above proposal.

[d3zd3z]

One of my concerns is with the notification mechanism in github. I've really tried to use it, but it seems to just like to randomly decide that I want to be notified about all kinds of things that aren't urgent or important to me, and I end up missing things, even things that are assigned to me. If anyone has any ideas on how to improve this, please let me know.

As far as not breaking existing behavior, I think this is good and important. We need to understand that mcuboot gets installed in systems for a very long time. People also develop new systems that they want to integrate into an existing environment, and don't want to have to deal with incompatibilities there. There are a couple of areas of compatibility that are important:

• Image format. This includes the header, the meaning of the TLVs and the magic number used to mark an image for update, as well as the current image good. The later could be changed in some circumstances when, for example, adding support for a new flash type that isn't currently supported. But, the existing TLVs are something we are stuck with. I'm sorry I missed allowing the TLV change to get in, and honestly, it needs to be reverted entirely, and done without breaking compatibility. I even suggested a way it could be done (continue using 0x22, but allow an additional one to specify a key size that isn't the default).
• API. Our API surface is really small, basically a small set of functions that can be linked into an app to allow for image updates. Unfortunately, this API isn't very flexible, and kind of hard codes how flash access is supposed to work. It shouldn't do anything with flash that isn't covered by the above image format, so it is possible to make some changes to this, since it goes with the app.

Things that are not covered by this are internal APIs and conventions and organization of the code. In addition, things stored in the flash that are an internal part of an in-process upgrade can be freely changed.

I'd suggest writing up a PR that adds this compatibility guaranty to the docs, as well as putting some aspect of it into the guides for contributors.

[nordicjm]

One of my concerns is with the notification mechanism in github. I've really tried to use it, but it seems to just like to randomly decide that I want to be notified about all kinds of things that aren't urgent or important to me, and I end up missing things, even things that are assigned to me. If anyone has any ideas on how to improve this, please let me know.

I would suggest for this having things automatically assigned depending upon code paths, i.e. for file changes in core it gets assigned to you, for file changes in mynewt paths it gets assigned to utzig, for zephyr it gets assigned to my and de-nordic, and others to whomever is the maintainer for those files, a bot can also add labels for each part that is changed.

[d3zd3z]

I would suggest for this having things automatically assigned depending upon code paths, i.e. for file changes in core it gets assigned to you, for file changes in mynewt paths it gets assigned to utzig, for zephyr it gets assigned to my and de-nordic, and others to whomever is the maintainer for those files, a bot can also add labels for each part that is changed.

I'm not real sure this project is high enough volume for that to really matter. And, for me, having something assigned to me seems to have little effect on how much the github notification system likes to spam me about the change.

[nordicjm]

I would suggest for this having things automatically assigned depending upon code paths, i.e. for file changes in core it gets assigned to you, for file changes in mynewt paths it gets assigned to utzig, for zephyr it gets assigned to my and de-nordic, and others to whomever is the maintainer for those files, a bot can also add labels for each part that is changed.

I'm not real sure this project is high enough volume for that to really matter. And, for me, having something assigned to me seems to have little effect on how much the github notification system likes to spam me about the change.

There should only be one email when a PR is opened and 1 email when comments are added or it is pushed, if you're assigned to the correct PRs then this should be expected? I.e. notification that a PR has been opened that you're a reviewer on and need to review, notification that a comment has been added/replied to and you need to check it or mark it as resolved, notification that they have pushed it and you need to recheck the PR.

[github-actions]

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.