-
Notifications
You must be signed in to change notification settings - Fork 683
DATACMNS-1601 - Improving the JavaDoc of the CrudRepository. #412
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
Conversation
Clarifying expectations and guarantees or lack thereof.
Document throws on save(…). Align documentation for reactive repositories.
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.
I found a lot of tiny stuff.
*/ | ||
Mono<T> findById(Publisher<ID> id); | ||
|
||
/** | ||
* Returns whether an entity with the id exists. | ||
* Returns whether an entity with the given {@code id} exists. |
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.
* Returns whether an entity with the given {@code id} exists. | |
* Returns whether an entity with the given {@literal id} exists. |
@@ -108,20 +109,29 @@ | |||
Flux<T> findAll(); | |||
|
|||
/** | |||
* Returns all instances with the given IDs. | |||
* Returns all instances of the type {@code T} with the given IDs. |
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.
* Returns all instances of the type {@code T} with the given IDs. | |
* Returns all instances of the type {@literal T} with the given IDs. |
*/ | ||
Flux<T> findAllById(Iterable<ID> ids); | ||
|
||
/** | ||
* Returns all instances of the type with the given IDs supplied by a {@link Publisher}. | ||
* Returns all instances of the type {@code T} with the given IDs supplied by a {@link Publisher}. |
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.
* Returns all instances of the type {@code T} with the given IDs supplied by a {@link Publisher}. | |
* Returns all instances of the type {@literal T} with the given IDs supplied by a {@link Publisher}. |
*/ | ||
Maybe<T> findById(Single<ID> id); | ||
|
||
/** | ||
* Returns whether an entity with the given id exists. | ||
* Returns whether an entity with the given {@code id} exists. |
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.
* Returns whether an entity with the given {@code id} exists. | |
* Returns whether an entity with the given {@literal id} exists. |
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.
I'm stopping commenting on this, since it is easier to find and fix within the IDE.
* | ||
* @param id must not be {@literal null}. | ||
* @return {@literal true} if an entity with the given id exists, {@literal false} otherwise. | ||
* @throws IllegalArgumentException if {@code id} is {@literal null}. | ||
* @return {@link Single} emitting {@literal true} if an entity with the given id exists, {@literal false} otherwise. |
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.
Ok it now gets really nitpicky but shouldn't it be TRUE
since it is the constant from the wrapper type, not the primitive?
If you agree there are more places like this.
* @return {@literal true} if an entity with the given id exists, {@literal false} otherwise. | ||
* @throws IllegalArgumentException if {@code id} is {@literal null}. | ||
* @return {@link Single} emitting {@literal true} if an entity with the given id exists, {@literal false} otherwise. | ||
* @throws IllegalArgumentException in case the given {@code id} is {@literal null}. |
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.
* @throws IllegalArgumentException in case the given {@code id} is {@literal null}. | |
* @throws IllegalArgumentException in case the given {@literal id} is {@literal null}. |
* @return {@literal true} if an entity with the given id exists, {@literal false} otherwise. | ||
* @throws IllegalArgumentException if {@code id} is {@literal null} | ||
* @return {@link Single} emitting {@literal true} if an entity with the given id exists, {@literal false} otherwise. | ||
* @throws IllegalArgumentException in case the given {@link Single id} is {@literal null}. |
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.
This should be either {@literal IllegalArgument}
or {@link IllegalArgument}
Same in many other places.
*/ | ||
Flowable<T> findAll(); | ||
|
||
/** | ||
* Returns all instances of the type with the given IDs. | ||
* Returns all instances of the type {@code T} with the given IDs. |
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.
We are mixing IDs and ids and I think it should be {@literal ids} referring to the parameter name.
*/ | ||
Flowable<T> findAllById(Iterable<ID> ids); | ||
|
||
/** | ||
* Returns all instances of the type with the given IDs. | ||
* Returns all instances of the type {@code T} with the given IDs supplied by a {@link Flowable}. |
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.
Same as above.
Clarifying expectations and guarantees or lack thereof. Original pull request: #412.
Document throws on save(…). Align documentation for reactive repositories. Original pull request: #412.
Clarifying expectations and guarantees or lack thereof. Original pull request: #412.
Document throws on save(…). Align documentation for reactive repositories. Original pull request: #412.
Clarifying expectations and guarantees or lack thereof. Original pull request: #412.
Document throws on save(…). Align documentation for reactive repositories. Original pull request: #412.
Document throws on save(…). Align documentation for reactive repositories. Original pull request: #412.
Document throws on save(…). Align documentation for reactive repositories. Original pull request: #412.
Document throws on save(…). Align documentation for reactive repositories. Original pull request: #412.
Updated to consistent literal usage. That's merged, polished, and backported now. |
Clarifying expectations and guarantees or lack thereof.
https://jira.spring.io/browse/DATACMNS-1601