Finish contributing updates; thanks @acarbonetto (#12981)

* Finish contributing updates; thanks @acarbonetto Signed-off-by: Stephen Crawford <[email protected]> * Fix indenting Signed-off-by: Stephen Crawford <[email protected]> * Readd build failure notes Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING with review tips Signed-off-by: Andrew Carbonetto <[email protected]> * Update CONTRIBUTING with review tips Signed-off-by: Andrew Carbonetto <[email protected]> * Update contributing comment for review comment Signed-off-by: Andrew Carbonetto <[email protected]> * Fix contruibuting doc Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING.md Co-authored-by: Owais Kazi <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING.md Co-authored-by: Owais Kazi <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING.md Co-authored-by: Owais Kazi <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING.md Co-authored-by: Owais Kazi <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING.md Co-authored-by: Owais Kazi <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING.md Co-authored-by: Owais Kazi <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING.md Co-authored-by: Owais Kazi <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> * Update CONTRIBUTING.md Co-authored-by: Owais Kazi <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> --------- Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Andrew Carbonetto <[email protected]> Signed-off-by: Stephen Crawford <[email protected]> Co-authored-by: Andrew Carbonetto <[email protected]> Co-authored-by: Owais Kazi <[email protected]> (cherry picked from commit 19d6c5c) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
opensearch-project · Apr 5, 2024 · 828ec4e · 828ec4e
1 parent 7e0ff5a
commit 828ec4e
Showing 1 changed file with 25 additions and 19 deletions.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -1,18 +1,18 @@
 - [Contributing to OpenSearch](#contributing-to-opensearch)
-  - [First Things First](#first-things-first)
-  - [Ways to Contribute](#ways-to-contribute)
+- [First Things First](#first-things-first)
+- [Ways to Contribute](#ways-to-contribute)
     - [Bug Reports](#bug-reports)
     - [Feature Requests](#feature-requests)
     - [Documentation Changes](#documentation-changes)
     - [Contributing Code](#contributing-code)
-  - [Developer Certificate of Origin](#developer-certificate-of-origin)
-  - [Changelog](#changelog)
-  - [Review Process](#review-process)
-  - [Troubleshooting Failing Builds](#troubleshooting-failing-builds)
+- [Developer Certificate of Origin](#developer-certificate-of-origin)
+- [Changelog](#changelog)
+- [Review Process](#review-process)
+    - [Tips for Success](#tips)
 
 # Contributing to OpenSearch
 
-OpenSearch is a community project that is built and maintained by people just like **you**. We're glad you're interested in helping out. There are several different ways you can do it, but before we talk about that, let's talk about how to get started.
+OpenSearch is a community project built and maintained by people just like **you**. We're glad you're interested in helping out. There are several different ways you can do it, but before we talk about that, let's talk about how to get started.
 
 ## First Things First
 
@@ -30,9 +30,9 @@ Ugh! Bugs!
 
 A bug is when software behaves in a way that you didn't expect and the developer didn't intend. To help us understand what's going on, we first want to make sure you're working from the latest version. Please make sure you're testing against the [latest version](https://github.com/opensearch-project/OpenSearch).
 
-Once you've confirmed that the bug still exists in the latest version, you'll want to check to make sure it's not something we already know about on the [open issues GitHub page](https://github.com/opensearch-project/OpenSearch/issues).
+Once you've confirmed that the bug still exists in the latest version, you'll want to check the bug is not something we already know about. A good way to figure this out is to search for your bug on the [open issues GitHub page](https://github.com/opensearch-project/OpenSearch/issues).
 
-If you've upgraded to the latest version and you can't find it in our open issues list, then you'll need to tell us how to reproduce it. To make the behavior as clear as possible, please provided your steps as `curl` commands which we can copy and paste into a terminal to run it locally, for example:
+If you've upgraded to the latest version and you can't find it in our open issues list, then you'll need to tell us how to reproduce it. To make the behavior as clear as possible, please provide your steps as `curl` commands which we can copy and paste into a terminal to run it locally, for example:
 
 ```sh
 # delete the index
@@ -47,11 +47,11 @@ curl -x PUT localhost:9200/test/test/1 -d '{
 curl ....
 ```
 
-Provide as much information as you can. You may think that the problem lies with your query, when actually it depends on how your data is indexed. The easier it is for us to recreate your problem, the faster it is likely to be fixed.
+Provide as much information as you can. You may think that the problem lies with your query, when actually it depends on how your data is indexed. The easier it is for us to recreate your problem, the faster it is likely to be fixed. It is generally always helpful to provide the basic details of your cluster configuration alongside your reproduction steps.
 
 ### Feature Requests
 
-If you've thought of a way that OpenSearch could be better, we want to hear about it. We track feature requests using GitHub, so please feel free to open an issue which describes the feature you would like to see, why you need it, and how it should work.
+If you've thought of a way that OpenSearch could be better, we want to hear about it. We track feature requests using GitHub, so please feel free to open an issue which describes the feature you would like to see, why you need it, and how it should work. After opening an issue, the fastest way to see your change made is to open a pull request following the requested changes you detailed in your issue. You can learn more about opening a pull request in the [contributing code section](#contributing-code).
 
 ### Documentation Changes
 
@@ -164,13 +164,19 @@ If we accept the PR, a [maintainer](MAINTAINERS.md) will merge your change and u
 
 If we reject the PR, 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 PR and we'll do our best to address any further points you raise.
 
-## Troubleshooting Failing Builds
+### Tips for Success (#tips)
 
-The OpenSearch testing framework offers many capabilities but exhibits significant complexity (it does lot of randomization internally to cover as many edge cases and variations as possible). Unfortunately, this posses a challenge by making it harder to discover important issues/bugs in straightforward way and may lead to so called flaky tests - the tests which flip randomly from success to failure without any code changes. 
+We have a lot of mechanisms to help expedite towards an accepted PR. Here are some tips for success:
+1. *Minimize BWC guarantees*: The first PR review cycle heavily focuses on the public facing APIs. This is what we have to "guarantee" as non-breaking for [bwc across major versions](./DEVELOPER_GUIDE.md#backwards-compatibility).
+2. *Do not copy non-compliant code*: Ensure that code is APLv2 compatible. This means that you have not copied any code from other sources unless that code is also APLv2 compatible.
+3. *Utilize feature flags*: Features that are safeguarded behind feature flags are more likely to be merged and backported, as they come with an additional layer of protection. Refer to this [example PR](https://github.com/opensearch-project/OpenSearch/pull/4959) for implementation details.
+4. *Use appropriate Java tags*:
+    - `@opensearch.internal`: Marks internal classes subject to rapid changes.
+    - `@opensearch.api`: Marks public-facing API classes with backward compatibility guarantees.
+    - `@opensearch.experimental`: Indicates rapidly changing [experimental code](./DEVELOPER_GUIDE.md#experimental-development).
+5. *Employ sandbox for significant core changes*: Any new features or enhancements that make changes to core classes (e.g., search phases, codecs, or specialized lucene APIs) are more likely to. be merged if they are sandboxed. This can only be enabled on the java CLI (`-Dsandbox.enabled=true`).
+6. *Micro-benchmark critical path*: This is a lesser known mechanism, but if you have critical path changes you're afraid will impact performance (the changes touch the garbage collector, heap, direct memory, or CPU) then including a [microbenchmark](https://github.com/opensearch-project/OpenSearch/tree/main/benchmarks) with your PR (and jfr or flamegraph results in the description) is a *GREAT IDEA* and will help expedite the review process.
+7. *Test rigorously*: Ensure thorough testing ([OpenSearchTestCase](./test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java) for unit tests, [OpenSearchIntegTestCase](./test/framework/src/main/java/org/opensearch/test/OpenSearchIntegTestCase.java) for integration & cluster tests, [OpenSearchRestTestCase](./test/framework/src/main/java/org/opensearch/test/rest/OpenSearchRestTestCase.java) for testing REST endpoint interfaces, and yaml tests with [ClientYamlTestSuiteIT](./rest-api-spec/src/yamlRestTest/java/org/opensearch/test/rest/ClientYamlTestSuiteIT.java) for REST integration tests)
+
+In general, adding more guardrails to your changes increases the likelihood of swift PR acceptance. We can always relax these guard rails in smaller followup PRs. Reverting a GA feature is much more difficult. Check out the [DEVELOPER_GUIDE](./DEVELOPER_GUIDE.md#submitting-changes) for more useful tips.
 
-If your pull request reports a failing test(s) on one of the checks, please:
- - look if there is an existing [issue](https://github.com/opensearch-project/OpenSearch/issues) reported for the test in question 
- - if not, please make sure this is not caused by your changes, run the failing test(s) locally for some time
- - if you are sure the failure is not related, please open a new [bug](https://github.com/opensearch-project/OpenSearch/issues/new?assignees=&labels=bug%2C+untriaged&projects=&template=bug_template.md&title=%5BBUG%5D) with `flaky-test` label
- - add a comment referencing the issue(s) or bug report(s) to your pull request explaining the failing build(s)
- - as a bonus point, try to contribute by fixing the flaky test(s)