Skip to content

Update the workflow sections #1158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 6, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
152 changes: 101 additions & 51 deletions docs/reference/contributing/guidelines/workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,90 +61,131 @@ If commits do not follow the above guidelines, we may request that you modify th

[How We Release Arm Mbed OS](../introduction/how-we-release-arm-mbed-os.html) explains Mbed OS versioning.

## Pull request types
## Pull request template
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The blog post says the template now "includes a note about reporting and questions and enhancements to the Forum," but I don't see that here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I think the blog post might have been a little confusing. When someone raises an issue that is not a bug then the git2jira bot closes the issue with a comment stating that questions or enhancements should be raised in the forums. It has nothing to do with PRs

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the issue template:
(which is not affected by this PR)
************************************** WARNING **************************************

The ciarcom bot parses this header automatically. Any deviation from the
template may cause the bot to automatically correct this header or may result in a
warning message, requesting updates.

Please ensure all sections of the template below are filled in and no changes
are made to the template format. Only bugs should be raised here as issues.
Questions or enhancements should instead be raised on our forums:
https://forums.mbed.com/ .


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying 👍
Should we mention somewhere (not in the PR section) that nonbug issues will be closed?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can submit Mbed OS bugs on the forums or directly on GitHub.

I'll send separate PR to change this in this workflow file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here #1161


We consider the following pull request types.
The following template is automatically provided when you raise a pull request against `mbed-os`. The details required depend on the type of pull request you create:

### Fix
### Description (*required*)

A bug fix is a change that fixes a specific defect in the codebase with backward compatibility. These are the highest priority because of the positive effect the change will have on users developing against the same code. A bug fix should be limited to restoring the documented or proven otherwise, originally intended behavior. Every bug fix should contain a test to verify results before and after the pull request. Bug fixes are candidates for patch releases. Large, nontrivial bug fixes approaching the size of refactors run the risk of being considered refactors themselves.
##### Summary of change (*What the change is for and why*)

Release: patch

### Refactor
##### Documentation (*Details of any document updates required*)

A refactor is a contribution that modifies the codebase without fixing a bug or changing the existing behavior. Examples of this are moving functions or variables between translation units, renaming source files or folders, scope modification for nonpublic code, documentation structure changes and test organization changes. There is always the risk that someone depended on the location or name before a refactor; therefore, these are lower in priority than bug fixes and might require detailed justification for the change. Refactors are candidates for feature releases.

Release: feature
----------------------------------------------------------------------------------------------------------------
### Pull request type (*required*)

### Target update
[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Updating target implementation (adding a new target or updating already supported target) is a change for a patch release.
----------------------------------------------------------------------------------------------------------------
### Test results (*required*)

A test report for the new target must be part of the pull request. The new target must pass all Mbed OS functional and system validation tests (using `mbed test` command) for the current Mbed OS major release, including all Mbed OS supported toolchains.
[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR


----------------------------------------------------------------------------------------------------------------
### Reviewers (*optional*)

Release: patch

### Functionality change
----------------------------------------------------------------------------------------------------------------
### Release Notes (*required for feature/major PRs*)

A functionality change can be any change in the functionality, including adding a new feature, a new method or a function. Software language does not matter.

A feature contribution contains a new API, capability or behavior. It does not break backward compatibility with existing APIs, capabilities or behaviors. New feature contributions are very welcome in Mbed OS. However, because they add capability to the codebase, a new feature may introduce bugs and a support burden. New features should also come with documentation, support for most targets and comprehensive test coverage. Feature PRs are treated cautiously, and new features require a new minor version for the codebase. Features are candidates for feature releases.
##### Summary of changes

Every pull request changing or adding functionality must contain a "Release notes" section describing the changes to users.
##### Impact of changes

It must contain:
##### Migration actions required

- A brief description of changes introduced, including justification.
- An analysis of effects: components affected and potential consequences for users.
- Migration guidance: actions for updating the current code. Please include code snippets to illustrate before and after the addition or change.

<span class="notes">**Note:** We may use this content in our official release notes.</span>
### Description field

For more details, please see the [pull request template addition for functional changes](#pull-request-template-functional-changes).
There are two parts to the description, both of which are required:

We initially implement new features on separate branches in the Mbed OS repository. Mbed OS maintainers create the new branches by following the naming convention: "feature-" prefix.
- The summary of the pull request clearly states the reason for the PR and what the changes involve.
- The documentation section requires you to state what, if any, documentation changes need to accompany the code changes.

Each feature has a tech lead. This person is responsible for:
### Pull request type

- Rebasing often to track master development.
- Reviewing any addition to the feature branch (approval required by the feature tech lead or another assigned person).
There are three pull request types, and these correspond to the three main categories specified in semantic versioning: patch, feature (minor) and major.

Release: feature
#### Patch update

### Documentation update
This can contain any of the following changes:

Documentation updates include changes to markdown files or Doxygen documentation (comment-only changes).
##### Bug Fix

A bug fix is a change that fixes a specific defect in the codebase with backward compatibility. These are the highest priority because of the positive effect the change will have on users developing against the same code. A bug fix should be limited to restoring the documented or proven otherwise, originally intended behavior. Every bug fix should contain a test to verify results before and after the pull request. Bug fixes are candidates for patch releases. Large, nontrivial bug fixes approaching the size of refactors run the risk of being considered refactors themselves.

##### Refactor

Release: patch
A refactor is a contribution that modifies the codebase without fixing a bug or changing the existing behavior. Examples of this are moving functions or variables between translation units, renaming source files or folders, scope modification for nonpublic code, documentation structure changes and test organization changes. There is always the risk that someone depended on the location or name before a refactor; therefore, these are lower in priority than bug fixes and might require detailed justification for the change. 

### Test update
##### Target update

Updating target implementation (adding a new target or updating already supported target) is a change for a patch release.

A test report for the new target must be part of the pull request. The new target must pass all Mbed OS functional and system validation tests (using `mbed test` command) for the current Mbed OS major release, including all Mbed OS supported toolchains.

##### Documentation update

Documentation updates include changes to markdown files or Doxygen documentation (comment-only changes).

##### Test update

Test updates include updates to a new test unit or test case.

Release: patch
#### Feature update

### Breaking change
This can be any change in the functionality, including adding a new feature, a new method or a function. Software language does not matter.

A breaking change is any change that results in breaking user space. It should have strong justification for us to consider it. Often, such changes can be backward compatible, for example, deprecating the old functionality and introducing the new replacement.
A feature contribution contains a new API, capability or behavior. It does not break backward compatibility with existing APIs, capabilities or behaviors. New feature contributions are very welcome in Mbed OS. However, because they add capability to the codebase, a new feature may introduce bugs and a support burden. New features should also come with documentation, support for most targets and comprehensive test coverage. Feature PRs are treated cautiously, and new features require a new minor version for the codebase. 

A contribution containing a breaking change is the most difficult PR to get merged. Any breaking changes in a codebase can have a large negative impact on any users of the codebase. Breaking changes are always limited to a major version release.
We initially implement new features on separate branches in the Mbed OS repository. Mbed OS maintainers or Mbed OS technical leads may create the new branches by following the naming convention: "feature-" prefix.

Every pull request with breaking change must contain a release notes section called "Release notes" to describe the changes to users.
Each feature has a Mbed OS technical lead. This person is responsible for:

It must contain:
- Rebasing often to track master development.
- Reviewing any addition to the feature branch.
- Approving all feature change pull requests.

- A brief description of changes introduced, including justification description.
- An analysis of effects: components affected, potential consequences for users.
- Migration guidance: actions for updating the current code. Please include code snippets to illustrate before and after the addition or change.
#### Major update

<span class="notes">**Note:** We may use this content in our official release notes.</span>
This is for breaking changes and should be rare. A breaking change is any change that results in breaking user space. It should have strong justification for us to consider it. Often, such changes can be backward compatible, for example, deprecating the old functionality and introducing the new replacement.

Please see the [Pull request template](../contributing/workflow.html#pull-request-template-breaking-changes).
A contribution containing a breaking change is the most difficult PR to get merged. Any breaking changes in a codebase can have a large negative effect on any users of the codebase. Breaking changes are always limited to a major version release.

A project technical lead and the Mbed OS technical lead must approve breaking change pull requests.

Release: major
### Test results

This section is to indicate what test results, if any, are required for the PR. The three options are:

- No tests required for this change (for example, a documentation-only update).
- Changes will be tested by existing `mbed-os` tests (Greentea or Unittest).
- Tests and results will be supplied as part of this PR. For this option, post the tests and test results
below the tick boxes.

### Reviewers

A bot automatically adds reviewers based on the files that are actually changed. However, this section gives you the option to specify additional, specific reviewers. Tag required reviewers here, such as @adbridge, @0xc0170.

### Release Notes

Every pull request changing or adding functionality must fill in the "Release notes" section. This applies to feature and major PRs. For both these types, you must complete the "Summary of changes" section. Provide a brief description of changes introduced, including justification.

For major PRs, it is also compulsory to complete the "Impact of changes" and "Migration actions required".

The impact of changes must contain an analysis of effects: components affected and potential consequences for users.

The migration actions required should describe how to migrate from a previous version of the code being changed to the new version. Please include code snippets to illustrate before and after the addition or change.

The release notes section is automatically pulled into the overall release notes for a feature or major release. This should be considered when you write the entries.

## GitHub pull requests workflow

Expand All @@ -166,37 +207,46 @@ All pull requests must be reviewed. The Arm Mbed CI bot determines the most suit

GitHub dismisses a reviewer's status after any change to the pull request commit history (such as adding a new commit or rebasing). Smaller changes, such as documentation edits or rebases on top of latest master, only require additional review by maintainers. Their approval is sufficient because a team assigned as a reviewer already approved the pull request.

Label: `needs: review`
Time: Three days for reviewers to leave feedback after the maintainers add the label.
- Label: `needs: review`.
- Time: Three days for reviewers to leave feedback after the maintainers add the label.

### The Continuous Integration (CI) testing

There are many [CI systems available](../contributing/ci.html) for testing Mbed OS pull requests and braches. Which CI tests we run against a particular pull request depends on the effect that it has on the code base. Irrespective of which CI tests run, Mbed OS has an all-green policy, meaning that all triggered CI jobs must pass before we merge the pull request.

- Label: `needs: CI`
- Label: `needs: CI`.
- Time: One day for CI to complete and report back results.

### Work needed

A pull request in the "work needed" state requires additional work due to failed tests, or rework as a result of the review. If a pull request is in this state, our maintainers request changes from the pull request author.

- Label: `needs: work`
- Label: `needs: work`.
- Time: Three days for the pull request author to action the review comments.

### Ready for integration

Maintainers merge pull requests during the internal gatekeeping meetings that occur three times a week. They can merge straightforward pull requests immediately.

- Label: `Ready for merge`
- Time: Two days
- Label: `Ready for merge`.
- Time: Two days.

### Releases

When we merge a pull request that we will publish in a patch release, we tag it with the specific patch release version. This is the release in which we first publish this pull request. For patch releases, we allow only bug fixes, new targets and enhancements to existing functionality. New features are only published in feature releases.

The release tag has the following format:
The release tag has the format:

*release-version: m.f.p*

Where:

- `m` is the major release.
- `f` is the feature release.
- `p` is the patch release.

*release-version: 5.f.p* - Where `f` is the feature release and `p` the patch release.
From time to time there may be additional suffixes added which could represent a release candidate or
alpha/beta release etc

## Additional labels

Expand Down