You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flink.apache.org by rm...@apache.org on 2019/06/21 14:36:40 UTC
[flink-web] 05/08: [FLINK-12605] Describe new contribution process
This is an automated email from the ASF dual-hosted git repository.
rmetzger pushed a commit to branch asf-site
in repository https://gitbox.apache.org/repos/asf/flink-web.git
commit 0d94ffa9737d53e8ee18f28d7df0757508bd817d
Author: Robert Metzger <rm...@apache.org>
AuthorDate: Thu May 23 14:06:39 2019 +0200
[FLINK-12605] Describe new contribution process
---
_data/i18n.yml | 4 +--
contributing/code-style-and-quality.md | 4 +--
...and-quality.md => code-style-and-quality.zh.md} | 4 +--
contributing/contribute-code.md | 8 ++---
contributing/contribute-jira-and-processes.md | 5 ----
contributing/reviewing-prs.md | 35 ++++++----------------
6 files changed, 19 insertions(+), 41 deletions(-)
diff --git a/_data/i18n.yml b/_data/i18n.yml
index c8841a4..7f06924 100644
--- a/_data/i18n.yml
+++ b/_data/i18n.yml
@@ -11,7 +11,7 @@ en:
community_project: Community & Project Info
how_to_contribute: How to Contribute
contribute_code: Contribute Code
- review_code: Review Code
+ review_code: Review Pull Requests
code_style_guide: Code Style and Quality Guide
contribute_docs: Contribute Documentation
contribute_website: Contribute to the Website
@@ -31,7 +31,7 @@ zh:
how_to_contribute: 如何参与贡献
how_to_contribute: How to Contribute
contribute_code: Contribute Code
- review_code: Review Code
+ review_code: Review Pull Requests
code_style_guide: Code Style and Quality Guide
contribute_docs: Contribute Documentation
contribute_website: Contribute to the Website
diff --git a/contributing/code-style-and-quality.md b/contributing/code-style-and-quality.md
index 942ab7d..087c2c4 100644
--- a/contributing/code-style-and-quality.md
+++ b/contributing/code-style-and-quality.md
@@ -28,7 +28,7 @@ title: "Code Style and Quality Guide (OUTDATED)"
### Tests
{:.no_toc}
-- **Tests need to pass**. Any pull request where the tests do not pass or which does not compile will not undergo any further review. We recommend to connect your personal GitHub accounts with [Travis CI](http://travis-ci.org/) (like the Flink GitHub repository). Travis will run tests for all tested environments whenever you push something into *your* GitHub repository. Please note the previous [comment about flaky tests]( {{site.base}}/contribute-code.html#verifying-the-compliance-of-yo [...]
+- **Tests need to pass**. Any pull request where the tests do not pass or which does not compile will not undergo any further review. We recommend to connect your personal GitHub accounts with [Travis CI](http://travis-ci.org/) (like the Flink GitHub repository). Travis will run tests for all tested environments whenever you push something into *your* GitHub repository.
- **Tests for new features are required**. All new features need to be backed by tests, *strictly*. It is very easy that a later merge accidentally throws out a feature or breaks it. This will not be caught if the feature is not guarded by tests. Anything not covered by a test is considered cosmetic.
@@ -37,7 +37,7 @@ title: "Code Style and Quality Guide (OUTDATED)"
### Documentation
{:.no_toc}
-- **Documentation Updates**. Many changes in the system will also affect the documentation (both Javadocs and the user documentation in the `docs/` directory). Pull requests and patches are required to update the documentation accordingly; otherwise the change can not be accepted to the source code. See the [Contribute documentation]({{site.base}}/contribute-documentation.html) guide for how to update the documentation.
+- **Documentation Updates**. Many changes in the system will also affect the documentation (both Javadocs and the user documentation in the `docs/` directory). Pull requests and patches are required to update the documentation accordingly; otherwise the change can not be accepted to the source code. See the [Contribute documentation]({{site.base}}/contributing/contribute-documentation.html) guide for how to update the documentation.
- **Javadocs for public methods**. All public methods and classes need to have Javadocs. Please write meaningful docs. Good docs are concise and informative. Please do also update Javadocs if you change the signature or behavior of a documented method.
diff --git a/contributing/code-style-and-quality.md b/contributing/code-style-and-quality.zh.md
similarity index 96%
copy from contributing/code-style-and-quality.md
copy to contributing/code-style-and-quality.zh.md
index 942ab7d..087c2c4 100644
--- a/contributing/code-style-and-quality.md
+++ b/contributing/code-style-and-quality.zh.md
@@ -28,7 +28,7 @@ title: "Code Style and Quality Guide (OUTDATED)"
### Tests
{:.no_toc}
-- **Tests need to pass**. Any pull request where the tests do not pass or which does not compile will not undergo any further review. We recommend to connect your personal GitHub accounts with [Travis CI](http://travis-ci.org/) (like the Flink GitHub repository). Travis will run tests for all tested environments whenever you push something into *your* GitHub repository. Please note the previous [comment about flaky tests]( {{site.base}}/contribute-code.html#verifying-the-compliance-of-yo [...]
+- **Tests need to pass**. Any pull request where the tests do not pass or which does not compile will not undergo any further review. We recommend to connect your personal GitHub accounts with [Travis CI](http://travis-ci.org/) (like the Flink GitHub repository). Travis will run tests for all tested environments whenever you push something into *your* GitHub repository.
- **Tests for new features are required**. All new features need to be backed by tests, *strictly*. It is very easy that a later merge accidentally throws out a feature or breaks it. This will not be caught if the feature is not guarded by tests. Anything not covered by a test is considered cosmetic.
@@ -37,7 +37,7 @@ title: "Code Style and Quality Guide (OUTDATED)"
### Documentation
{:.no_toc}
-- **Documentation Updates**. Many changes in the system will also affect the documentation (both Javadocs and the user documentation in the `docs/` directory). Pull requests and patches are required to update the documentation accordingly; otherwise the change can not be accepted to the source code. See the [Contribute documentation]({{site.base}}/contribute-documentation.html) guide for how to update the documentation.
+- **Documentation Updates**. Many changes in the system will also affect the documentation (both Javadocs and the user documentation in the `docs/` directory). Pull requests and patches are required to update the documentation accordingly; otherwise the change can not be accepted to the source code. See the [Contribute documentation]({{site.base}}/contributing/contribute-documentation.html) guide for how to update the documentation.
- **Javadocs for public methods**. All public methods and classes need to have Javadocs. Please write meaningful docs. Good docs are concise and informative. Please do also update Javadocs if you change the signature or behavior of a documented method.
diff --git a/contributing/contribute-code.md b/contributing/contribute-code.md
index f14219c..29ad2b3 100644
--- a/contributing/contribute-code.md
+++ b/contributing/contribute-code.md
@@ -156,11 +156,11 @@ This means agreeing on the scope and implementation approach of a change.
- Is this a contribution just for the sake of getting a commit in an open source project (fixing typos, style changes merely for taste reasons)
- There is **consensus** on how to solve the problem. This includes considerations such as
- API and data backwards compatibility and migration strategies
- - testing strategies
- - impact on Flink's build time
- - dependencies and their licenses
+ - Testing strategies
+ - Impact on Flink's build time
+ - Dependencies and their licenses
-Large changes might require a [Flink Improvement Proposal (FLIP)](https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals) or a discussion on the [dev mailing list]( {{ site.base }}/community.html#mailing-lists ) to reach agreement or consensus.
+Large or controversial changes might require a [Flink Improvement Proposal (FLIP)](https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals) or a discussion on the [dev mailing list]( {{ site.base }}/community.html#mailing-lists ) to reach agreement or consensus.
Once all requirements for the ticket are met, a committer will assign somebody to the *`Assignee`* field of the ticket to work on it.
Only committers have the permission to assign somebody.
diff --git a/contributing/contribute-jira-and-processes.md b/contributing/contribute-jira-and-processes.md
deleted file mode 100644
index acc8619..0000000
--- a/contributing/contribute-jira-and-processes.md
+++ /dev/null
@@ -1,5 +0,0 @@
----
-title: "Jira and Processes in Flink"
----
-
-How is Flink using Jira?
diff --git a/contributing/reviewing-prs.md b/contributing/reviewing-prs.md
index b988095..a767437 100644
--- a/contributing/reviewing-prs.md
+++ b/contributing/reviewing-prs.md
@@ -4,11 +4,6 @@ title: "How to Review a Pull Request"
<hr />
-TODO:
-- LICENSE files
-- documentation
--
-
This guide is for all committers and contributors that want to help with reviewing code contributions. Thank you for your effort - good reviews are one the most important and crucial parts of an open source project. This guide should help the community to make reviews such that:
* Contributors have a good contribution experience.
@@ -22,7 +17,7 @@ This guide is for all committers and contributors that want to help with reviewi
## Review Checklist
-Every review needs to check the following five aspects. We encourage to check these aspects in order, to avoid spending time on detailed code quality reviews when there is no consensus yet whether a feature or change should actually be added.
+Every review needs to check the following five aspects. **We encourage to check these aspects in order, to avoid spending time on detailed code quality reviews when there is no consensus yet whether a feature or change should actually be added.**
### 1. Is the Contribution Well-Described?
@@ -36,24 +31,11 @@ Changes that require longer descriptions are ideally based on a prior design dis
### 2. Is There Consensus that the Change or Feature Should Go into Flink?
-For bug fixes, this needs to be checked only in case it requires bigger changes or might break existing programs and setups.
-
-Ideally, this question can be directly answered from a Jira issue or a dev-list discussion, except in cases of bug fixes and small lightweight additions/extensions. In that case, this question can be immediately marked as resolved. For pull requests that are created without prior consensus, this question needs to be answered as part of the review.
-
-The decision whether the change should go into Flink needs to take the following aspects into consideration:
-
-* Does the contribution alter the behavior of features or components in a way that it may break previous users’ programs and setups? If yes, there needs to be a discussion and agreement that this change is desirable.
-* Does the contribution conceptually fit well into Flink? Is it too much of a special case such that it makes things more complicated for the common case, or bloats the abstractions / APIs?
-* Does the feature fit well into Flink's architecture? Will it scale and keep Flink flexible for the future, or will the feature restrict Flink in the future?
-* Is the feature a significant new addition (rather than an improvement to an existing part)? If yes, will the Flink community commit to maintaining this feature?
-* Does the feature produce added value for Flink users or developers? Or does it introduce the risk of regression without adding relevant user or developer benefit?
-* Could the contribution live in another repository, e.g., [Apache Bahir](https://bahir.apache.org) or another external repository?
+This question can be directly answered from the linked Jira issue. For pull requests that are created without prior consensus, a [discussion in Jira to seek consensus]({{ site.base }}/contributing/contribute-code.html#consensus) will be needed.
-All of these questions should be answerable from the description/discussion in Jira and Pull Request, without looking at the code.
-**A feature, improvement, or bug fix is approved once one committer accepts it and no committer disagrees (lazy consensus).**
+For `[hotfix]` pull requests, consensus needs to be checked in the pull request.
-In case of diverging opinions, the discussion should be moved to the respective Jira issue or to the dev mailing list and continued until consensus is reached. If the change is proposed by a committer, it is best-practice to seek the approval of another committer.
-----
@@ -86,23 +68,24 @@ We recommend to check this before diving into the details of commenting on indiv
This is the detailed code review of the actual changes, covering:
-* Are the changes doing what is described in the design document or PR description?
+* Are the changes doing what is described in the Jira ticket or design document?
* Does the code follow the right software engineering practices? Is the code correct, robust, maintainable, testable?
* Are the changes performance aware, when changing a performance sensitive part?
* Are the changes sufficiently covered by tests?
* Are the tests executing fast, i.e., are heavy-weight integration tests only used when necessary?
* Does the code format follow Flink’s checkstyle pattern?
* Does the code avoid to introduce additional compiler warnings?
+* If dependencies have been changed, were the NOTICE files updated?
-Some code style guidelines can be found in the [Flink Code Style Page]({{ site.baseurl }}/contribute-code.html#code-style)
+Code guidelines can be found in the [Flink Code Style and Quality Guide]({{ site.baseurl }}/contributing/code-style-and-quality.html).
----
-### 6. Are English and Chinese documentation updated?
+### 6. Are the English and Chinese documentation updated?
-If the pull request introduces a new feature, the feature should be documented. The Flink community is maintaining both English and Chinese documents. So both English and Chinese documentation should be updated. If you are not familiar with Chinese language, please open a JIRA tagged with the `chinese-translation` component for Chinese documentation translation and link it with current JIRA issue. If you are familiar with Chinese language, you are encouraged to update both sides in one p [...]
+If the pull request introduces a new feature, the feature should be documented. The Flink community is maintaining both an English and a Chinese documentation. So both documentations should be updated. If you are not familiar with Chinese language, please open a JIRA tagged with the `chinese-translation` component for Chinese documentation translation and link it with current JIRA issue. If you are familiar with Chinese language, you are encouraged to update both sides in one pull request.
-See more about how to [contribute documentation](https://flink.apache.org/contribute-documentation.html).
+See more about how to [contribute documentation]({{ site.baseurl }}/contributing/contribute-documentation.html).
## Review with the @flinkbot