-
Notifications
You must be signed in to change notification settings - Fork 178
Contributing - update licensing and tips #871
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
Conversation
To be done: license introduction vs licence page (@AnotherButler if you can rewrite these so they compliment each other). The goal is to have contributing page as intro for licensing and license page to contain licenses examples and more detail information. |
|
||
### Types of contributions | ||
|
||
There are a few categories a contribution may fall under. The type of contribution will impact how it is incorporated into Mbed, as explained here: Each type has different risks and benefits. When contributing it’s important to not mix types, rather, create multiple contributions if needed. Once a contribution is accepted it will appear in the next release based on type of contribution. |
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.
Colon after here? Seems wrong ?
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.
Also 'based on the type of contribution'
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.
Fixed
|
||
There are a few categories a contribution may fall under. The type of contribution will impact how it is incorporated into Mbed, as explained here: Each type has different risks and benefits. When contributing it’s important to not mix types, rather, create multiple contributions if needed. Once a contribution is accepted it will appear in the next release based on type of contribution. | ||
|
||
Types of contributions are described in [here](../contributing/workflow.html) |
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.
Don't need the 'in'
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.
Fixed
|
||
### How to contribute | ||
|
||
Mbed OS has a team of people called maintainers who will help move contributions along, providing guidance and direction. This team is responsible for helping you get your changes in, as well as controlling the overall quality and consistency of the software. Mbed software is developed on GitHub and contributions are accepted in the form of a pull requests. Before any contributions are accepted to any Mbed software there must be a review by at least one other developer experienced with the functionality. For contributions that span multiple functionalities, multiple reviewers may be necessary. Once reviewed the changes will be tested as part of a larger system. The testing includes but is not limited to: functional correctness, static analysis, integration with other parts of the system, code style or formatting and regressions such as code size increase or performance degredation. If any of the testing fails, more work will be needed before the contribution is accepted. |
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've already said previously that Mbed software is developed on Github, not sure we need to say it again? Could just say 'Contribution are accepted in the form of pull requests' ?
|
||
### Licensing | ||
|
||
A license is the contract between the author permitting the use of software to others. It specifies what you can and cannot do when receiving the software. It provides protections for both the user and owner of the software. In an Mbed project, the full terms of the license can be found in a file named LICENSE. Additionally, all source files must contain the SPDX identifier as a comment at the beginning of the file. |
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.
A license is the contract between the author permitting the use of software to others
This doesn't make sense
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.
It provides protections
It provides protection
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.
A license is the contract between the author permitting the use of software to others
@AnotherButler Can you edit this one ?
|
||
A license is the contract between the author permitting the use of software to others. It specifies what you can and cannot do when receiving the software. It provides protections for both the user and owner of the software. In an Mbed project, the full terms of the license can be found in a file named LICENSE. Additionally, all source files must contain the SPDX identifier as a comment at the beginning of the file. | ||
|
||
Note that one repository may contain multiple, independent codebases, each with their own license. If you are integrating two libraries with different licenses, it is important that each library retains its original license. In the case of a repository having software with multiple licenses, the contribution will be made according to the license of the file the contribution modifies. By creating a pull request on GitHub, you are agreeing to license your contributions under the same license as the original code. Is commonly called "inbound=outbound". This enables contributions to happen in a quick and effortless way and encourages collaboration. |
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.
Is commonly called
This is commonly referred to as
|
||
For new Mbed projects, we suggest adopting the Apache 2.0 license. Note that any Mbed software release under a permissive license cannot accept any code that is licensed under a "copyleft" license. Doing so would prevent us from continuing to distribute our code under the permissive license. You are welcome to use Mbed software with copyleft licenses, as long as the rules of that license are followed. | ||
|
||
More detailed licenses description can be found in the [guidelines/contributing](../contributing/guidelines/license.html). |
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.
More detailed licenses description
A more detailed description on licenses
@@ -93,13 +93,13 @@ We consider the following pull request types. | |||
|
|||
#### Fix | |||
|
|||
A bug fix is a backward-compatible internal change that fixes incorrect behavior. | |||
A bug fix is a change that fixes a specific defect in the codebase with the backward compatibility. These are the highest priority because the positive impact 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. Bug fixes are candidates for patch releases. |
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.
codebase with the backward compatibility
You don't need 'the' and backward should be backwards
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.
because the positive impact the change
because of the positive impact the change
|
||
Release: patch | ||
|
||
#### Refactor | ||
|
||
Refactors are intended for feature releases, if they are not fixing specific issues, because they can introduce new issues. | ||
A refactor is a contribution that modifies the codebase without fixing a bug or changing the existing behavior. This some examples would be 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 are lower in priority than bug fixes and might require detailed justification for the change. Refactors are candidates for feature releases. |
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 some examples
Examples of this
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.
therefore are lower in priority
therefore these are lower in priority
|
||
New features target feature releases. A new feature can only be integrated if the feature supports most of the targets (if it requires new target HAL implementation). | ||
Any change in the functionality, it can be adding new feature or adding new method or function. It does not matter if it is C++, C or Python. |
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.
adding new feature or adding new method or function
adding a new feature, adding a new method or a function
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.
Thanks , will fix all !
Too late for 5.11; we'll do it next week |
@adbridge I fixed them all
OK, as soon as this can go in. Webteam can use at least references to the current docs to do their updates. We have all docs pages in place just content needs updating (this PR). |
|
Now that it's all in one place I'm questioning the structure; I'll look into it next week. |
we would like to restructure the workflow document (it's growing day by day). Will wait for feedback |
|
||
### How to contribute | ||
|
||
Mbed OS has a team of people called maintainers who will help move contributions along, providing guidance and direction. This team is responsible for helping you get your changes in, as well as controlling the overall quality and consistency of the software. Contributions are accepted in the form of a pull requests. Before any contributions are accepted to any Mbed software there must be a review by at least one other developer experienced with the functionality. For contributions that span multiple functionalities, multiple reviewers may be necessary. Once reviewed the changes will be tested as part of a larger system. The testing includes but is not limited to: functional correctness, static analysis, integration with other parts of the system, code style or formatting and regressions such as code size increase or performance degredation. If any of the testing fails, more work will be needed before the contribution is accepted. |
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.
in the form of a pull requests
Should be:
in the form of pull requests
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.
fixed
- The maintainers and reviewers are your friends. At times, programming can be very personal. However, it's important to realize that we all share a common goal, and that honest feedback is constructive feedback. | ||
- Code consistency and maintainability is more important than functionality. The existing style of a codebase overrules any personal preference. | ||
- Larger contributions take longer to be accepted than smaller contributions. The best contributions are small and purposeful, achieving a single goal. You may be asked to split up a contribution if it contains multiple unrelated changes. | ||
- Consistency is an important aspect of a codebase. To ensure consistency in Mbed OS code, we have created contributing guidelines. Any contribution to Mbed OS needs to meet the following criteria: |
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.
I wonder if line 37 should be moved into this list.
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.
will fix
|
||
Release: patch | ||
|
||
#### Refactor | ||
|
||
Refactors are intended for feature releases, if they are not fixing specific issues, because they can introduce new issues. | ||
A refactor is a contribution that modifies the codebase without fixing a bug or changing the existing behavior. Examples of this would be 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. |
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.
If a refactor is made in a backwards-compatble way, can they be targeted for patch releases?
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.
What about if a refactor fixes a bug?
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.
Either they fix a bug separately or will go with refactor. Everything should be backward compatible (only exceptionally not)
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.
So making sure, if a the only way to fix a particular bug is with a refactor, that PR will have to come into the next major release?
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.
It should
Co-Authored-By: 0xc0170 <[email protected]>
Co-Authored-By: 0xc0170 <[email protected]>
Co-Authored-By: 0xc0170 <[email protected]>
Co-Authored-By: 0xc0170 <[email protected]>
Co-Authored-By: 0xc0170 <[email protected]>
@cmonr should be all fixed (used github beta feature to accept suggestions, I can rebase later entire branch) |
Co-Authored-By: 0xc0170 <[email protected]>
I'm just wanting to make sure, because we've had instances in the past where we've done the opposite. I'm not sure the suggestions as placed above follow our previous guidelines, in particular this comment chain: ARMmbed/mbed-os#8273 (comment) Becoming more strict would make the small updates that we see from TLS, COAP or other libraries be pushed into the next feature release, when we've done the opposite before. |
@@ -93,13 +93,13 @@ We consider the following pull request types. | |||
|
|||
#### Fix | |||
|
|||
A bug fix is a backward-compatible internal change that fixes incorrect behavior. | |||
A bug fix is a change that fixes a specific defect in the codebase with backwards compatibility. These are the highest priority because of the positive impact 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. Bug fixes are candidates for patch releases. |
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.
A bug fix is a change that fixes a specific defect in the codebase with backwards compatibility. These are the highest priority because of the positive impact 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. Bug fixes are candidates for patch releases. | |
A bug fix is a change that fixes a specific defect in the codebase with backwards compatibility. These are the highest priority because of the positive impact 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. Bug fixes are candidates for patch releases. Large, non-trivial bug fixes approaching the size of refactors run the risk of being considered refactors themselves. | |
@0xc0170 | |
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.
+1, added
Y'all okay with me merging this into development and then opening a new PR? I want to potentially move things about, and that's harder to do in an existing PR. |
OK with me. |
Our contributing page was missing details and was duplicated in two places.
This should bring it all to one place, here in the docs.
Note, I am not certain I got new links correct, please fix them and let me know
This should deprecate os.mbed.com contributing page (it should point to this one, no duplication).
Target to have in 5.11.
Review @ARMmbed/mbed-os-maintainers @AnotherButler