honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add section on reviews to CONTRIBUTING.md (#57046) 

The review phase is an important part of contributing to Elasticsearch, but we
do not mention it in our instructions to contributors. This commit adds some
notes on how contributions are reviewed and describes some common reasons for
rejection.

Co-authored-by: Colin Goodheart-Smithe <colings86@users.noreply.github.com>
This commit is contained in:
David Turner 2020-07-03 08:03:03 +01:00
parent 1133c29ce9
commit df555bb470
1 changed files with 70 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
CONTRIBUTING.md
View File

@ -34,14 +34,17 @@ Open an issue on our [issues list](https://github.com/elastic/elasticsearch/issu
Contributing code and documentation changes
-------------------------------------------
If you have a bugfix or new feature that you would like to contribute to Elasticsearch, please find or open an issue about it first. Talk about what you would like to do. It may be that somebody is already working on it, or that there are particular issues that you should know about before implementing the change.
If you would like to contribute a new feature or a bug fix to Elasticsearch,
please discuss your idea first on the Github issue. If there is no Github issue
for your idea, please open one. It may be that somebody is already working on
it, or that there are particular complexities that you should know about before
starting the implementation. There are often a number of ways to fix a problem
and it is important to find the right approach before spending time on a PR
that cannot be merged.
We enjoy working with contributors to get their code accepted. There are many approaches to fixing a problem and it is important to find the best approach before writing too much code.
Note that it is unlikely the project will merge refactors for the sake of refactoring. These
types of pull requests have a high cost to maintainers in reviewing and testing with little
to no tangible benefit. This especially includes changes generated by tools. For example,
converting all generic interface instances to use the diamond operator.
We add the `help wanted` label to existing Github issues for which community
contributions are particularly welcome, and we use the `good first issue` label
to mark issues that we think will be suitable for new contributors.
The process for contributing to any of the [Elastic repositories](https://github.com/elastic/) is similar. Details for individual projects can be found below.
@ -427,6 +430,66 @@ that are part of this project but not production code. The canonical example
of this is `junit`.</dd>
</dl>
Reviewing and accepting your contribution
-----------------------------------------
We review every contribution carefully to ensure that the change is of high
quality and fits well with the rest of the Elasticsearch codebase. If accepted,
we will merge your change and usually take care of backporting it to
appropriate branches ourselves.
We really appreciate everyone who is interested in contributing to
Elasticsearch and regret that we sometimes have to reject contributions even
when they might appear to make genuine improvements to the system. Reviewing
contributions can be a very time-consuming task, yet the team is small and our
time is very limited. In some cases the time we would need to spend on reviews
would outweigh the benefits of a change by preventing us from working on other
more beneficial changes instead.
Please discuss your change in a Github issue before spending much time on its
implementation. We sometimes have to reject contributions that duplicate other
efforts, take the wrong approach to solving a problem, or solve a problem which
does not need solving. An up-front discussion often saves a good deal of wasted
time in these cases.
We normally immediately reject isolated PRs that only perform simple
refactorings or otherwise "tidy up" certain aspects of the code. We think the
benefits of this kind of change are very small, and in our experience it is not
worth investing the substantial effort needed to review them. This especially
includes changes suggested by tools.
We sometimes reject contributions due to the low quality of the submission
since low-quality submissions tend to take unreasonable effort to review
properly. Quality is rather subjective so it is hard to describe exactly how to
avoid this, but there are some basic steps you can take to reduce the chances
of rejection. Follow the guidelines listed above when preparing your changes.
You should add tests that correspond with your changes, and your PR should pass
affected test suites too. It makes it much easier to review if your code is
formatted correctly and does not include unnecessary extra changes.
We sometimes reject contributions if we find ourselves performing many review
iterations without making enough progress. Some iteration is expected,
particularly on technically complicated changes, and there's no fixed limit on
the acceptable number of review cycles since it depends so much on the nature
of the change. You can help to reduce the number of iterations by reviewing
your contribution yourself or in your own team before asking us for a review.
You may be surprised how many comments you can anticipate and address by taking
a short break and then carefully looking over your changes again.
We expect you to follow up on review comments somewhat promptly, but recognise
that everyone has many priorities for their time and may not be able to respond
for several days. We will understand if you find yourself without the time to
complete your contribution, but please let us know that you have stopped
working on it. We will try to send you a reminder if we haven't heard from you
in a while, but may end up closing your PR if you do not respond for too long.
If your contribution is rejected we will close the pull request with a comment
explaining why. This decision isn't always final: if you feel we have
misunderstood your intended change or otherwise think that we should reconsider
then please continue the conversation with a comment on the pull request and
we'll do our best to address any further points you raise.
Contributing as part of a class
-------------------------------
In general Elasticsearch is happy to accept contributions that were created as