Skip to content

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

Merged
merged 16 commits into from
Dec 18, 2018

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Dec 12, 2018

Our contributing page was missing details and was duplicated in two places.

This should bring it all to one place, here in the docs.

  • updated PR types decription
  • add new sections to the contributing page - what types of contributions do we have, how to contribute, licenses introduction and some useful tips

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

@0xc0170
Copy link
Contributor Author

0xc0170 commented Dec 12, 2018

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.
Copy link
Contributor

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 ?

Copy link
Contributor

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'

Copy link
Contributor Author

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)
Copy link
Contributor

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'

Copy link
Contributor Author

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.
Copy link
Contributor

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. 
Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor Author

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. 
Copy link
Contributor

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).
Copy link
Contributor

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.
Copy link
Contributor

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

Copy link
Contributor

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.
Copy link
Contributor

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

Copy link
Contributor

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.
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks , will fix all !

@iriark01
Copy link
Contributor

Too late for 5.11; we'll do it next week

@0xc0170
Copy link
Contributor Author

0xc0170 commented Dec 13, 2018

@adbridge I fixed them all

Too late for 5.11; we'll do it next week

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).

@0xc0170
Copy link
Contributor Author

0xc0170 commented Dec 13, 2018

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).

cc @JoeTheGuitarist

@iriark01
Copy link
Contributor

Now that it's all in one place I'm questioning the structure; I'll look into it next week.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Dec 14, 2018

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.
Copy link
Contributor

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

Copy link
Contributor Author

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:
Copy link
Contributor

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.

Copy link
Contributor Author

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.
Copy link
Contributor

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?

Copy link
Contributor

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?

Copy link
Contributor Author

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)

Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should

@0xc0170
Copy link
Contributor Author

0xc0170 commented Dec 14, 2018

@cmonr should be all fixed (used github beta feature to accept suggestions, I can rebase later entire branch)

@cmonr
Copy link
Contributor

cmonr commented Dec 14, 2018

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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1, added

@iriark01
Copy link
Contributor

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.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Dec 18, 2018

OK with me.

@iriark01 iriark01 merged commit cbac570 into ARMmbed:development Dec 18, 2018
@0xc0170 0xc0170 deleted the fix_contributing branch December 19, 2018 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants