-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
docs: Add development branch section #9119
base: mbedtls-3.6
Are you sure you want to change the base?
Conversation
2fd6b16
to
e813ae3
Compare
CONTRIBUTING.md
Outdated
@@ -10,7 +10,7 @@ More details on all of these points may be found in the sections below. | |||
- [Sign-off](#license-and-copyright): all commits must be signed off. | |||
- [Tests](#tests): please ensure the PR includes adequate tests. | |||
- [Changelog](#documentation): if needed, please provide a changelog entry. | |||
- [Backports](#long-term-support-branches): provide a backport if needed (it's fine to wait until the main PR is accepted). | |||
- [Backports](#long-term-support-branches): Priority is given to the LTS branch over the development branch. After the PR has been integrated into the LTS branch, please merge it into the development branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Err, not really.
development
is where all development happens. Some PRs will be backported into the LTS branches, but not all. So all PRs must first start on development
; it is not true that priority is given to LTS branches. And each PR targets only one branch (which is different to some other projects, such as OpenSSL, where a single PR is often merged to multiple applicable branches).
What is true is that we won't merge a PR to development
if we want backport PRs and they are not present. Very specifically, gatekeepers (should) merge all PRs at the same time.
Now, some people will create the backports at the same time as the main PR. This means extra work if there are changes required.
The original phrase it's fine to wait until the main PR is accepted
is correct. It does not say merged
.
It is fine to wait until we are happy with a PR before creating backports. But it's usually best to start with a PR against development
(obviously there are special cases of issues that are only in an LTS branch, but ignoring that for the sake of a generally-applicable rule) and then create backport PRs if required/requested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tom-cosgrove-arm Thank you for clarifying a lot more about the workflow. Your comments are very good, and I think they should be in CONTRIBUTING.md
. I have submitted a new commit, adding most of your comments.
Sorry for the forced-commit. The PR is totally different from the original change, and I thought the original commit was not necessary to keep it.
Clarify development branch workflow to help out in submitting PRs. Signed-off-by: Javier Tia <[email protected]>
e813ae3
to
99ac255
Compare
development branch is where all development happens. Some PRs will be backported into the LTS branches, but not all. So all PRs must first start with development. We won't merge a PR into development if we want backport PRs and they are not present. Very specifically, gatekeepers should merge all PRs at the same time. | ||
|
||
Now, some people will create the backports at the same time as the main PR. If changes are required, this means extra work. | ||
|
||
It is fine to wait until we are happy with a PR before creating backports. But it's usually best to start with a PR against development (obviously there are special cases of issues that are only in an LTS branch, but ignoring that for the sake of a generally applicable rule) and then create backport PRs if required or requested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all seems like it's spending a lot of effort providing information that people don't really need. development
is the default branch for Git and on GitHub, so that's what you'll get when you clone the repository and that's what you'll target when you make a pull request. Backports need a discussion, but they already have their own section.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We clearly need something, since at least one person didn't find what we currently have straightforward enough. However, I'm not sure that my rambling GitHub comments are the best fit for the documentation here: I'm very happy for others to suggest wording
Description
development branch is an important part of the PR workflow, and it deserves its own section to explain its use in detail.
PR checklist
Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")
Notes for the submitter
Please refer to the contributing guidelines, especially the
checklist for PR contributors.
Help make review efficient: