Skip to content

Commit 36dc170

Browse files
opensearch-trigger-bot[bot]github-actions[bot]acarbonettoowaiskazi19
authored
Finish contributing updates; thanks @acarbonetto (#12981) (#13088)
* Finish contributing updates; thanks @acarbonetto * Fix indenting * Readd build failure notes * Update CONTRIBUTING with review tips * Update CONTRIBUTING with review tips * Update contributing comment for review comment * Fix contruibuting doc * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md * Update CONTRIBUTING.md --------- (cherry picked from commit 19d6c5c) Signed-off-by: Stephen Crawford <steecraw@amazon.com> Signed-off-by: Andrew Carbonetto <andrewc@bitquilltech.com> Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Andrew Carbonetto <andrewc@bitquilltech.com> Co-authored-by: Owais Kazi <owaiskazi19@gmail.com>
1 parent 1dde447 commit 36dc170

File tree

1 file changed

+25
-19
lines changed

1 file changed

+25
-19
lines changed

CONTRIBUTING.md

+25-19
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
- [Contributing to OpenSearch](#contributing-to-opensearch)
2-
- [First Things First](#first-things-first)
3-
- [Ways to Contribute](#ways-to-contribute)
2+
- [First Things First](#first-things-first)
3+
- [Ways to Contribute](#ways-to-contribute)
44
- [Bug Reports](#bug-reports)
55
- [Feature Requests](#feature-requests)
66
- [Documentation Changes](#documentation-changes)
77
- [Contributing Code](#contributing-code)
8-
- [Developer Certificate of Origin](#developer-certificate-of-origin)
9-
- [Changelog](#changelog)
10-
- [Review Process](#review-process)
11-
- [Troubleshooting Failing Builds](#troubleshooting-failing-builds)
8+
- [Developer Certificate of Origin](#developer-certificate-of-origin)
9+
- [Changelog](#changelog)
10+
- [Review Process](#review-process)
11+
- [Tips for Success](#tips)
1212

1313
# Contributing to OpenSearch
1414

15-
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.
15+
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.
1616

1717
## First Things First
1818

@@ -30,9 +30,9 @@ Ugh! Bugs!
3030

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

33-
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).
33+
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).
3434

35-
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:
35+
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:
3636

3737
```sh
3838
# delete the index
@@ -47,11 +47,11 @@ curl -x PUT localhost:9200/test/test/1 -d '{
4747
curl ....
4848
```
4949

50-
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.
50+
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.
5151

5252
### Feature Requests
5353

54-
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.
54+
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).
5555

5656
### Documentation Changes
5757

@@ -164,13 +164,19 @@ If we accept the PR, a [maintainer](MAINTAINERS.md) will merge your change and u
164164

165165
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.
166166

167-
## Troubleshooting Failing Builds
167+
### Tips for Success (#tips)
168168

169-
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.
169+
We have a lot of mechanisms to help expedite towards an accepted PR. Here are some tips for success:
170+
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).
171+
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.
172+
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.
173+
4. *Use appropriate Java tags*:
174+
- `@opensearch.internal`: Marks internal classes subject to rapid changes.
175+
- `@opensearch.api`: Marks public-facing API classes with backward compatibility guarantees.
176+
- `@opensearch.experimental`: Indicates rapidly changing [experimental code](./DEVELOPER_GUIDE.md#experimental-development).
177+
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`).
178+
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.
179+
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)
180+
181+
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.
170182

171-
If your pull request reports a failing test(s) on one of the checks, please:
172-
- look if there is an existing [issue](https://github.com/opensearch-project/OpenSearch/issues) reported for the test in question
173-
- if not, please make sure this is not caused by your changes, run the failing test(s) locally for some time
174-
- 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
175-
- add a comment referencing the issue(s) or bug report(s) to your pull request explaining the failing build(s)
176-
- as a bonus point, try to contribute by fixing the flaky test(s)

0 commit comments

Comments
 (0)