Skip to content

Commit 9cc8959

Browse files
Adding code guidelines (#502)
* Adding code guidelines to DEVELOPER_GUIDE md Signed-off-by: Martin Gaievski <[email protected]> * Adding guidelines to the table of content Signed-off-by: Martin Gaievski <[email protected]> * Added section with spotlessApply and javadoc commands Signed-off-by: Martin Gaievski <[email protected]> --------- Signed-off-by: Martin Gaievski <[email protected]>
1 parent 628cb64 commit 9cc8959

File tree

1 file changed

+119
-0
lines changed

1 file changed

+119
-0
lines changed

DEVELOPER_GUIDE.md

+119
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,14 @@
1717
- [Supported configurations](#supported-configurations)
1818
- [Submitting Changes](#submitting-changes)
1919
- [Building On Lucene Version Updates](#building-on-lucene-version-updates)
20+
- [Code Guidelines](#code-guidelines)
21+
- [Class and package names](#class-and-package-names)
22+
- [Modular code](#modular-code)
23+
- [Documentation](#documentation)
24+
- [Code style](#code-style)
25+
- [Style and Formatting Check](#style-and-formatting-check)
26+
- [Tests](#tests)
27+
- [Outdated or irrelevant code](#outdated-or-irrelevant-code)
2028

2129
# Developer Guide
2230

@@ -338,3 +346,114 @@ through the same build issue.
338346
3. Head over to your OpenSearch cloned repo root directory
339347
1. `./gradlew publisToMavenLocal`
340348
4. Finally run `./gradlew build` from the neural search repo
349+
350+
## Code Guidelines
351+
352+
### Class and package names
353+
354+
Class names should use `CamelCase`.
355+
356+
Try to put new classes into existing packages if package name abstracts the purpose of the class.
357+
358+
Example of good class file name and package utilization:
359+
360+
`src/main/java/org/opensearch/neuralsearch/processor/factory/RerankProcessorFactory.java`
361+
362+
following naming needs improvement, it creates unnecessary package and uses underscores case for file name
363+
364+
`src/main/java/org/opensearch/neuralsearch/rerank_factory/rerank_processor_factory.java`
365+
366+
### Modular code
367+
368+
Try to organize code into small classes and methods with a single concise purpose. It's preferable to have multiple small
369+
methods rather than a long single one and does everything.
370+
371+
### Documentation
372+
373+
Document you code. That includes purpose of new classes, every public method and code sections that have critical or non-trivial
374+
logic (check this example https://github.com/opensearch-project/neural-search/blob/main/src/main/java/org/opensearch/neuralsearch/query/NeuralQueryBuilder.java#L238).
375+
376+
When you submit a feature PR, please submit a new
377+
[documentation issue](https://github.com/opensearch-project/documentation-website/issues/new/choose). This is a path for the documentation to be published as part of https://opensearch.org/docs/latest/ documentation site.
378+
379+
Please be prepared to provide any additional guidance (like example of query request/response, details of API parameters etc.) for the team doing the documentation.
380+
381+
### Code style
382+
383+
For the most part, we're using common conventions for Java projects. Here are a few things to keep in mind.
384+
385+
1. Use descriptive names for classes, methods, fields, and variables.
386+
2. Avoid abbreviations unless they are widely accepted
387+
3. Use `final` on all method arguments unless it's absolutely necessary
388+
4. Wildcard imports are not allowed.
389+
5. Static imports are preferred over qualified imports when using static methods
390+
6. Prefer creating non-static public methods whenever possible. Avoid static methods in general, as they can often serve as shortcuts.
391+
Static methods are acceptable if they are private and do not access class state.
392+
7. Use functional programming style inside methods unless it's a performance critical section.
393+
8. For parameters of lambda expression please use meaningful names instead of shorten cryptic ones.
394+
9. Use Optional for return values if the value may not be present. This should be preferred to returning null.
395+
10. Do not create checked exceptions, and do not throw checked exceptions from public methods whenever possible. In general, if you call a method with a checked exception, you should wrap that exception into an unchecked exception.
396+
11. Throwing checked exceptions from private methods is acceptable.
397+
12. Use String.format when a string includes parameters, and prefer this over direct string concatenation. Always specify a Locale with String.format;
398+
as a rule of thumb, use Locale.ROOT.
399+
13. Prefer Lombok annotations to the manually written boilerplate code
400+
14. When throwing an exception, avoid including user-provided content in the exception message. For secure coding practices,
401+
limit exception messages to the bare minimum and log additional details to the application logs, as user input could be maliciously crafted.
402+
403+
Please check [Java Language Formatting Guidelines](##Java Language Formatting Guidelines) section for more details.
404+
405+
### Style and Formatting Check
406+
As part of our continuous integration (CI) pipeline, we check code formatting and style. Failed checks may block your PR from being merged. To avoid this, run the following command locally before submitting your PR:
407+
408+
```bash
409+
./gradlew spotlessApply
410+
```
411+
412+
To verify that all required public Java documentation is present, run:
413+
414+
```bash
415+
./gradlew javadoc
416+
```
417+
418+
This command will identify missing or incomplete documentation. Common warnings include:
419+
420+
- missing documentation for public methods
421+
- missing parameter descriptions
422+
- missing documentation for public fields
423+
424+
Example warning:
425+
426+
```
427+
/neural-search/build/generated/sources/delombok/java/main/org/opensearch/neuralsearch/processor/combination/ArithmeticMeanScoreCombinationTechnique.java:18: warning: no comment
428+
public static final String TECHNIQUE_NAME = "arithmetic_mean";
429+
```
430+
Documentation requirements:
431+
432+
- all public methods must have descriptions that explain:
433+
- main responsibility of the method
434+
- any side effects (like mutating input objects)
435+
- any exceptional conditions or error cases
436+
- all parameters in public methods must have descriptions
437+
- public fields should be documented with their purpose and usage
438+
439+
Fix any new warnings before submitting your PR to ensure proper code documentation.
440+
441+
### Tests
442+
443+
Write unit and integration tests for your new functionality.
444+
445+
Unit tests are preferred as they are cheap and fast, try to use them to cover all possible
446+
combinations of parameters. Utilize mocks to mimic dependencies.
447+
448+
Integration tests should be used sparingly, focusing primarily on the main (happy path) scenario or cases where extensive
449+
mocking is impractical. Include one or two unhappy paths to confirm that correct response codes are returned to the user.
450+
Whenever possible, favor scenarios that do not require model deployment. If model deployment is necessary, use an existing
451+
model, as tests involving new model deployments are the most resource-intensive.
452+
453+
If your changes could affect backward compatibility, please include relevant backward compatibility tests along with your
454+
PR. For guidance on adding these tests, refer to the [Backwards Compatibility Testing](#backwards-compatibility-testing) section in this guide.
455+
456+
### Outdated or irrelevant code
457+
458+
Do not submit code that is not used or needed, even if it's commented. We rely on github as version control system, code
459+
can be restored if needed.

0 commit comments

Comments
 (0)