Skip to content
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

Add contribution guidelines. #1348

Merged
merged 1 commit into from
Sep 16, 2016
Merged

Add contribution guidelines. #1348

merged 1 commit into from
Sep 16, 2016

Conversation

tilmannOSG
Copy link

@tilmannOSG tilmannOSG commented Sep 13, 2016

Add contribution guidelines and DCO.

The current content in CONTRIBUTING.md is largely based on the "Patch Submission Process" Wiki page.

This commit also moves the DCO from the Wiki into the source tree.

In addition, clarify in README.md that contributions need to be licensed under the Apache License 2.0 and signed with the DCO.

JerryScript-DCO-1.0-Signed-off-by: Tilmann Scheller t.scheller@samsung.com


It happens all the time, for many reasons, and not necessarily because the code is bad. Take the feedback, adapt your code, and try again. Remember, the ultimate goal is to preserve the quality of the code and maintain the focus of the Project through intensive review.

Maintainers and Committers typically have to process a lot of submissions, and the time for any individual response is generally limited. If the reason for rejection is unclear, please ask for more information to the Maintainers and Committers.
Copy link
Member

Choose a reason for hiding this comment

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

(syntax) ?from? the Maintainers and Committers

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

@akosthekiss akosthekiss added the documentation Related to documentation label Sep 14, 2016
* Listen and be open to all different opinions.
* Help each other.

Changes are submitted via pull requests and only the Maintainers and Committers should approve or reject the pull request.
Copy link
Contributor

Choose a reason for hiding this comment

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

The difference between Maintainers and Committers is unclear for me at this point.

Copy link
Author

Choose a reason for hiding this comment

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

The key difference is that only a Maintainer can do the final approval of patches (see https://github.com/Samsung/jerryscript/wiki/Governance#maintainer). However, Committers are still strongly encouraged to review and approve patches (even if ultimately the approval is not binding and the Maintainer has the last word).

Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO, this should be mentioned in this MD file too.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed.

@tilmannOSG
Copy link
Author

Addressed the review feedback. Now starting to merge #1347 into this PR.

@tilmannOSG
Copy link
Author

Merge of #1347 completed, please review b0fd1b5 :)
Tried to keep as little as possible in DCO.md.

Copy link
Member

@akosthekiss akosthekiss left a comment

Choose a reason for hiding this comment

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

Almost OK with me. See (hopefully) final comments.


> (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project, under the same open source license.

We have the same requirements for using the signed-off-by process as the Linux kernel.
Copy link
Member

Choose a reason for hiding this comment

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

I'd drop this line and the following rest from here. It is duplicated in CONTRIBUTING.md and it's a better fit there anyway

Copy link
Author

@tilmannOSG tilmannOSG Sep 16, 2016

Choose a reason for hiding this comment

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

I don't think we can drop this, DCO.md specifies the terms and conditions of the DCO and the format of the "Signed-off-by" line is part of that. That's what I was referring to when I mentioned that I tried to make DCO.md as small as possible :)
Ideally we would just include DCO.md into CONTRIBUTING.md but unfortunately GitHub does not support this and there are no plans to add that in the future (too many security implications).
I'm afraid we just have to live with the small amount of duplication here.

@@ -38,8 +38,11 @@ For additional information see [Getting Started](docs/01.GETTING-STARTED.md).
- [API Example](docs/03.API-EXAMPLE.md)
- [Internals](docs/04.INTERNALS.md)

## Contributing
The project can only accept contributions which are licensed under the [Apache License 2.0](LICENSE) and are signed according to the JerryScript [Developer's Certificate of Origin](DCO.md). For further information please see [CONTRIBUTING.md](CONTRIBUTING.md).
Copy link
Member

Choose a reason for hiding this comment

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

just a stylistic suggestion: please see [Contribution Guidelines](CONTRIBUTING.md). ?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed :)

@@ -0,0 +1,75 @@
## Patch Submission Process
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need a top-level heading # Contribution Guidelines here?

Copy link
Author

Choose a reason for hiding this comment

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

Fixed as well.

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

LGTM

The current content in CONTRIBUTING.md is largely based on the "Patch Submission Process" Wiki page.

This commit also moves the DCO from the Wiki into the source tree.

In addition, clarify in README.md that contributions need to be licensed under the Apache License 2.0 and signed with the DCO.

JerryScript-DCO-1.0-Signed-off-by: Tilmann Scheller t.scheller@samsung.com
@akosthekiss
Copy link
Member

LGTM

@tilmannOSG tilmannOSG merged commit ea96430 into jerryscript-project:master Sep 16, 2016
@tilmannOSG tilmannOSG deleted the add-contribution-guidelines branch September 16, 2016 13:51
bsdelf pushed a commit to bsdelf/jerryscript that referenced this pull request Sep 18, 2016
The current content in CONTRIBUTING.md is largely based on the "Patch Submission Process" Wiki page.

This commit also moves the DCO from the Wiki into the source tree.

In addition, clarify in README.md that contributions need to be licensed under the Apache License 2.0 and signed with the DCO.

JerryScript-DCO-1.0-Signed-off-by: Tilmann Scheller t.scheller@samsung.com
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants