workflow: add 3rd party updates as an exception #1193
Conversation
| @@ -52,7 +52,7 @@ Pull requests on GitHub have to meet the following requirements to keep the code | |||
| - All new features and enhancements require documentation, tests and user guides for us to accept them. Please link each pull request to all relevant documentation and test pull requests. | |||
| - Avoid merging commmits. (Always rebase when possible.) | |||
| - Comment in the pull request on every change (rebase or new commits). This helps reviewers to be up to date with changes | |||
| - Pull requests should fix a bug, add a feature or refactor. | |||
| - Pull requests should fix a bug, add a feature or refactor. The only exception are third-party party version updates (for instance MbedTLS or Nanostack releases for Mbed OS). | |||
There was a problem hiding this comment.
Maybe also add:
These should considered as a library update and contain an updated version, good level of detail documenting what has changed and any impacts on the user.
0xc0170
force-pushed
the
fix_workflow_3rdparty
branch
2 times, most recently
from
4e294be to
18b766a
Compare
|
@kjbracey-arm @SeppoTakalo @Patater Please review - as you have updated already external libraries, you got an experience. Have we missed anything? |
|
I think this is acceptable exception. However, I'm not sure whether we maintain changelog for Nanostack, but at least it points to some known release tag in the repository. Short changelog should of course be provided. |
| @@ -52,7 +52,8 @@ Pull requests on GitHub have to meet the following requirements to keep the code | |||
| - All new features and enhancements require documentation, tests and user guides for us to accept them. Please link each pull request to all relevant documentation and test pull requests. | |||
| - Avoid merging commmits. (Always rebase when possible.) | |||
| - Comment in the pull request on every change (rebase or new commits). This helps reviewers to be up to date with changes | |||
| - Pull requests should fix a bug, add a feature or refactor. | |||
| - Pull requests should fix a bug, add a feature or refactor. | |||
| The only exception are third-party version updates (for instance MbedTLS or Nanostack releases for Mbed OS). These updates should provide release notes for updated version with documentation (a reference to external changelog or release notes). | |||
There was a problem hiding this comment.
Respect the trademark.
- MbedTLS
+ Mbed TLSThere was a problem hiding this comment.
We don't always have a reference to an external changelog or release notes, depending on the library being updated and if the release is a pre-release version.
- These updates should provide release notes for updated version with documentation (a reference to external changelog or release notes).
+ These updates should provide Mbed OS release notes in the pull request description, or link to an external changelog or release notes.
There was a problem hiding this comment.
I'm fine with this.
rather to dedicate it entire section (also add how to add subtree - as nanostack is doing ?).
I was considering mentioning that if only as an exception to "avoid merging commits". Don't have any strong feelings either way though. Might be too specialised.
Should we make this change? - The only exception are third-party version updates
+ The only exceptions are third-party version updatesor perhaps this one? - The only exception are third-party version updates
+ The only exception is third-party version updates |
|
I'll rebase and fix soon |
Updates like MbedTLS might contain more than just a bugfix. To keep it simple, they should be sent as one PR - version update. Splitting this type of updates makes it difficult to maintain. It should be stated what version we are getting and a reference to release notes (changelog could be - what this brings in).
Fix phrasing, as requested in comments.
Fix phrasing for agreement.
0xc0170
force-pushed
the
fix_workflow_3rdparty
branch
from
849c74f to
a80422b
Compare
|
All updated. This should be now ready for integration |
Apply changes from PR #1193 to v5.15.
Updates like MbedTLS might contain more than just a bugfix. To keep it simple, they should be sent as one PR - version update. Splitting this type of updates makes it difficult to maintain.
This was missing the guidelines - version updates should not be separated. Would it be sufficient to note there as it is now, or rather to dedicate it entire section (also add how to add subtree - as nanostack is doing ?).
@ARMmbed/mbed-os-maintainers Please review