-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clean up doc/dev
documentation files
#6195
base: dev-2.x
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,96 +1,96 @@ | ||||||
# Codestyle | ||||||
# Code Style | ||||||
|
||||||
We use the following code conventions for [Java](#Java) and [JavaScript](#JavaScript). | ||||||
|
||||||
## Java | ||||||
|
||||||
The OpenTripPlanner Java code style is revised in OTP v2.2. We use the | ||||||
[Prettier Java](https://github.com/jhipster/prettier-java) as is. Maven is setup to | ||||||
[Prettier Java](https://github.com/jhipster/prettier-java) as is. Maven is set up to | ||||||
run `prettier-maven-plugin`. A check is run in the CI build, which fails the build preventing | ||||||
merging a PR if the code-style is incorrect. | ||||||
merging a PR if the code style is incorrect. | ||||||
|
||||||
There is two ways to format the code before checkin it in. You may run a normal build with maven - | ||||||
it takes a bit of time, but reformat the entire codebase. Only code you have changed should be | ||||||
formatted, since the existing code is already formatted. The second way is to set up prettier and | ||||||
There are two ways to format the code before checking it in. You may run a normal build with | ||||||
Maven—it takes a bit of time, but reformat the entire codebase. Only code you have changed should be | ||||||
formatted, since the existing code is already formatted. The second way is to set up Prettier and | ||||||
run it manually or hick it into your IDE, so it runs every time a file is changed. | ||||||
|
||||||
### How to run Prettier with Maven | ||||||
### How to Run Prettier with Maven | ||||||
|
||||||
Prettier will automatically format all code in the Maven "validate" phase, which runs before the test, package, and install phases. So formatting will happen for example when you run: | ||||||
Prettier will automatically format all code in the Maven "validate" phase, which runs before the | ||||||
test, package, and install phases. So formatting will happen for example when you run: | ||||||
|
||||||
``` | ||||||
```shell | ||||||
% mvn test | ||||||
``` | ||||||
|
||||||
You can manually run _only_ the formatting process with: | ||||||
|
||||||
``` | ||||||
```shell | ||||||
% mvn prettier:write | ||||||
``` | ||||||
|
||||||
To skip the prettier formating use profile `prettierSkip`: | ||||||
To skip the Prettier formating, use the profile `prettierSkip`: | ||||||
|
||||||
``` | ||||||
```shell | ||||||
% mvn test -P prettierSkip | ||||||
``` | ||||||
|
||||||
The check for formatting errors use profile `prettierCheck`: | ||||||
To check for formatting errors, use the profile `prettierCheck`: | ||||||
|
||||||
``` | ||||||
```shell | ||||||
% mvn test -P prettierCheck | ||||||
``` | ||||||
|
||||||
The check is run by the CI server and will fail the build if the code is incorrectly formatted. | ||||||
|
||||||
### IntellJ and Code Style Formatting | ||||||
### IntelliJ and Code Style Formatting | ||||||
|
||||||
You should use the prettier Maven plugin to reformat the code or run prettier with Node(faster). | ||||||
You should use the Prettier Maven plugin to reformat the code or run Prettier with Node (faster). | ||||||
|
||||||
The prettier does NOT format the doc and markdown files, only Java code. So, for other files you should | ||||||
use the _project_ code-style. It is automatically imported when you first open the project. But, if | ||||||
you have set a custom code-style in your settings (as we used until OTP v2.1), then you need to | ||||||
change to the _Project_ Code Style. Open the `Preferences` from the menu and select _ | ||||||
Editor > Code Style_. Then select **Project** in the \_Scheme drop down. | ||||||
Prettier does _not_ format the doc and Markdown files—only Java code. So, for other files, you | ||||||
should use the _project_ code style. It is automatically imported when you first open the project. | ||||||
But, if you have set a custom code style in your settings (as we used until OTP v2.1), then you need | ||||||
to change to the _Project_ code style. Open the `Preferences` from the menu and select _Editor > | ||||||
Code Style_. Then select **Project** in the \_Scheme drop down. | ||||||
|
||||||
#### Run Prettier Maven Plugin as an external tool in IntelliJ | ||||||
|
||||||
You can run the Prettier Maven plugin as an external tool in IntelliJ. Set it up as an | ||||||
`External tool` and assign a key-shortcut to the tool execution. | ||||||
`External tool` and assign a keyboard shortcut to the tool execution. | ||||||
|
||||||
![External Tool Dialog](../images/ExternalToolDialog.png) | ||||||
|
||||||
``` | ||||||
```text | ||||||
Name: Prettier Format Current File | ||||||
Program: mvn | ||||||
Arguments: prettier:write -Dprettier.inputGlobs=$FilePathRelativeToProjectRoot$ | ||||||
Working Directory: $ProjectFileDir$ | ||||||
``` | ||||||
> **Tip!** Add a unused key shortcut to execute the external tool, then you can use the old | ||||||
> short-cut to format other file types. | ||||||
|
||||||
> **Tip!** Add an unused key shortcut to execute the external tool. Then you can use the old | ||||||
> shortcut to format other file types. | ||||||
#### Install File Watchers plugin in IntelliJ | ||||||
|
||||||
#### Install File Watchers Plugin in IntelliJ | ||||||
You can also configure IntelliJ to run Prettier every time IntelliJ saves a Java file. But if you | ||||||
are editing the file at the same time, you will get a warning that the file in memory and the file | ||||||
on disk both changed, and asked to select one of them. | ||||||
|
||||||
You can also configure IntelliJ to run the prettier every time IntelliJ save a Java file. But, | ||||||
if you are editing the file at the same time you will get a warning that the file in memory and the | ||||||
file on disk both changed - and asked to select one of them. | ||||||
1. In the menu, open _Preferences..._ and select _Plugins_. | ||||||
2. Search for "File Watchers" in the Marketplace. | ||||||
3. Run _Install_. | ||||||
|
||||||
1. In the menyopen _Prefernces..._ and select _Plugins_. | ||||||
2. Search for "File Watchers" in Marketplace | ||||||
3. Run _Install_ | ||||||
##### Configure File Watchers | ||||||
|
||||||
##### Configure File Watcher | ||||||
You can run Prettier upon every file save in IntelliJ using the File Watchers plugin. There are | ||||||
several ways to set it up. Below is how to configure it using Maven to run the formatter. The Maven | ||||||
way works without any installation of other components but might be a bit slow. So you might want to | ||||||
install [prettier-java](https://github.com/jhipster/prettier-java/) in your shell and run it | ||||||
instead. | ||||||
|
||||||
You can run Prettier on every file save in Intellij using the File Watcher plugin. There is several | ||||||
ways to set it up. Below is hwo to configure it using Maven to run the formatter. The Maven way work | ||||||
without any installation of other components, but might be a bit slow. So, you might want to install | ||||||
[prettier-java](https://github.com/jhipster/prettier-java/) in your shell and run it instead. | ||||||
|
||||||
``` | ||||||
```text | ||||||
Name: Format files with Prettier | ||||||
File type: Java | ||||||
File Type: Java | ||||||
Scope: Project Files | ||||||
Program: mvn | ||||||
Arguments: prettier:write -Dprettier.inputGlobs=$FilePathRelativeToProjectRoot$ | ||||||
|
@@ -99,43 +99,44 @@ Working Directory: $ProjectFileDir$ | |||||
|
||||||
### Other IDEs | ||||||
|
||||||
We do not have support for other IDEs at the moment. If you use another editor and make one please | ||||||
We do not have support for other IDEs at the moment. If you use another editor and make one, please | ||||||
feel free to share it. | ||||||
|
||||||
### Sorting Class Members | ||||||
|
||||||
Some of the classes in OTP have a lot of fields and methods. Keeping members sorted reduce the merge | ||||||
Some of the classes in OTP have a lot of fields and methods. Keeping members sorted reduces merge | ||||||
conflicts. Adding fields and methods to the end of the list will cause merge conflicts more often | ||||||
than inserting methods and fields in an ordered list. Fields and methods can be sorted in "feature" | ||||||
sections or alphabetically, but stick to it and respect it when adding new methods and fields. | ||||||
than will inserting methods and fields in an ordered list. Fields and methods can be sorted in | ||||||
"feature" sections or alphabetically, but stick to it and respect it when adding new methods and | ||||||
fields. | ||||||
|
||||||
The provided formatter will group class members in this order: | ||||||
|
||||||
1. Getter and Setter methods are kept together | ||||||
2. Overridden methods are kept together | ||||||
3. Dependent methods are sorted in a breadth-first order. | ||||||
1. Getter and setter methods are kept together. | ||||||
2. Overridden methods are kept together. | ||||||
3. Dependent methods are sorted in breadth-first order. | ||||||
4. Members are sorted like this: | ||||||
1. `static` `final` fields (constants) | ||||||
1. `static final` fields (constants) | ||||||
2. `static` fields (avoid) | ||||||
3. instance fields | ||||||
4. static initializer | ||||||
5. class initializer | ||||||
6. constructor | ||||||
3. Instance fields | ||||||
4. Static initializers | ||||||
5. Class initializers | ||||||
6. Constructors | ||||||
7. `static` factory methods | ||||||
8. `public` methods | ||||||
9. getter and setters | ||||||
8. `private`/package methods | ||||||
10. `private` enums (avoid `public`) | ||||||
11. interfaces | ||||||
12. `private static` classes (avoid `public`) | ||||||
13. instance classes (avoid) | ||||||
9. Getter and setters | ||||||
10. `private`/package methods | ||||||
11. `private` enums (avoid `public`) | ||||||
12. Interfaces | ||||||
13. `private static` classes (avoid `public`) | ||||||
14. Instance classes (avoid) | ||||||
Comment on lines
-126
to
+132
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like when I applied #6125 (comment), it somehow messed up the order numbers. |
||||||
|
||||||
### JavaDoc Guidlines | ||||||
### Javadoc Guidelines | ||||||
|
||||||
As a matter of [policy](http://github.com/opentripplanner/OpenTripPlanner/issues/93), all new | ||||||
methods, classes, and fields should include comments explaining what they are for and any other | ||||||
pertinent information. For Java code, the comments should follow industry standards. It is best to | ||||||
provide comments that not only explain *what* you did but also *why you did it* while providing some | ||||||
provide comments that explain not only *what* you did but also *why you did it* while providing some | ||||||
context. Please avoid including trivial Javadoc or the empty Javadoc stubs added by IDEs, such as | ||||||
`@param` annotations with no description. | ||||||
|
||||||
|
@@ -144,41 +145,41 @@ context. Please avoid including trivial Javadoc or the empty Javadoc stubs added | |||||
- Contract of the method | ||||||
- Input domain for which the logic is designed | ||||||
- Range of outputs produced from valid inputs | ||||||
- Is behavior undefined or will fail when conditions are not met | ||||||
- Are null values allowed as inputs | ||||||
- Will null values occur as outputs (what do they mean) | ||||||
- Is behavior undefined or will the method fail when conditions are not met? | ||||||
- Are null values allowed as inputs? | ||||||
- Will null values occur as outputs (and what do they mean)? | ||||||
- Invariants that hold if the preconditions are met | ||||||
- Concurrency | ||||||
- Is method thread-safe | ||||||
- Is the method thread-safe? | ||||||
- Usage constraints for multi-threaded use | ||||||
- On classes: | ||||||
- Initialization and teardown process | ||||||
- Can instance be reused for multiple operations, or should it be discarded | ||||||
- Is it immutable or should anything be treated as immutable | ||||||
- Is it a utility class of static methods that should not be instantiated | ||||||
- Can an instance be reused for multiple operations, or should it be discarded? | ||||||
- Is it immutable, or should anything be treated as immutable? | ||||||
- Is it a utility class of static methods that should not be instantiated? | ||||||
|
||||||
### Annotations | ||||||
|
||||||
- On methods: | ||||||
- Method should be marked as `@Nullable` if they can return null values | ||||||
- Method should be marked as `@Nullable` if they can return null values. | ||||||
- Method parameters should be marked as `@Nullable` if they can take null values. | ||||||
- On fields: | ||||||
- Fields should be marked as `@Nullable` if they are nullable. | ||||||
|
||||||
Use of `@Nonnull` annotation is not allowed. It should be assumed methods/parameters/fields | ||||||
are non-null if they are not marked as `@Nullable`. However, there are places where the | ||||||
`@Nullable` annotation is missing even if it should have been used. Those can be updated | ||||||
to use the `@Nullable` annotation. | ||||||
Use of `@NonNull` annotation is not allowed. It should be assumed methods/parameters/fields are | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The javax annotation is |
||||||
non-null if they are not marked as `@Nullable`. However, there are places where the `@Nullable` | ||||||
annotation is missing even if it should have been used. Those can be updated to use the `@Nullable` | ||||||
annotation. | ||||||
|
||||||
## JavaScript | ||||||
|
||||||
As of #206, we | ||||||
follow [Crockford's JavaScript code conventions](http://javascript.crockford.com/code.html). Further | ||||||
As of [#206](https://github.com/opentripplanner/OpenTripPlanner/issues/206), we follow | ||||||
[Crockford's JavaScript code conventions](http://javascript.crockford.com/code.html). Further | ||||||
guidelines include: | ||||||
|
||||||
* All .js source files should contain one class only | ||||||
* Capitalize the class name, as well as the source file name (a la Java) | ||||||
* Include the namespace definition in each and every file: `otp.namespace("otp.configure");` | ||||||
* Capitalize the class name, as well as the source file name (a la Java). | ||||||
* Include the namespace definition in each and every file: `otp.namespace("otp.configure");`. | ||||||
* Include a class comment. For example, | ||||||
|
||||||
```javascript | ||||||
|
@@ -187,8 +188,8 @@ guidelines include: | |||||
* | ||||||
* Purpose is to allow a generic configuration object to be read via AJAX/JSON, and inserted into an | ||||||
* Ext Store | ||||||
* The implementation is TriMet route map specific...but replacing ConfigureStore object (or member | ||||||
* variables) with another implementation, will give this widget flexibility for other uses beyond | ||||||
* The implementation is TriMet route map-specific...but replacing ConfigureStore object (or member | ||||||
* variables) with another implementation will give this widget flexibility for other uses beyond | ||||||
* the iMap. | ||||||
* | ||||||
* @class | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong, but I think this word is not needed here.