- Notifications
You must be signed in to change notification settings - Fork 698
Full editing pass for Spring Data Commons #281
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
Thanks a lot. I filed DATACMNS-1291 to track this pull request. |
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.
Thanks a lot for the editing. Doing a full pass on the documentation is a valuable exercise and I found some spots we need to add further documentation or align wording. Documentation reads more polished now.
I left a few comments (some as reminders for me) on the pull request.
src/main/asciidoc/auditing.adoc Outdated
==== | ||
| ||
As you can see, the annotations can be applied selectively, depending on which information you'd like to capture. For the annotations capturing the points in time can be used on properties of type JodaTimes `DateTime`, legacy Java `Date` and `Calendar`, JDK8 date/time types as well as `long`/`Long`. | ||
As you can see, the annotations can be applied selectively, depending on which information you want to capture. The annotations capturing when changes were made can be used on properties of type `JodaTimes`, `DateTime`, legacy Java `Date` and `Calendar`, JDK8 date and time types, and `long` or `Long`. |
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.
JodaTime is a library. Maybe we should state it as Joda-Time (that's how they write it on http://www.joda.org/joda-time/)?
Example: "… can be used on properties of type Joda-Time, DateTime
, …"
== Introduction | ||
| ||
This chapter will give you an introduction to Query by Example and explain how to use Examples. | ||
This chapter provides an introduction to Query by Example and explains how to use the Examples. |
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.
Example in Query-by-Example is not specific to code samples here but rather a concept. Is it still "use the Examples"? Or should we rather rewrite the last part to something like "and explains how to use it."?
==== | ||
| ||
By default the `ExampleMatcher` will expect all values set on the probe to match. If you want to get results matching any of the predicates defined implicitly, use `ExampleMatcher.matchingAny()`. | ||
By default, the `ExampleMatcher` expect alls values set on the probe to match. If you want to get results matching any of the predicates defined implicitly, use `ExampleMatcher.matchingAny()`. |
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.
Is it alls or all?
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 was aiming for "expects all" and missed. Thanks for catching it. :)
src/main/asciidoc/repositories.adoc Outdated
==== | ||
| ||
NOTE: We also provide persistence technology-specific abstractions like e.g. `JpaRepository` or `MongoRepository`. Those interfaces extend `CrudRepository` and expose the capabilities of the underlying persistence technology in addition to the rather generic persistence technology-agnostic interfaces like e.g. CrudRepository. | ||
NOTE: We also provide persistence technology-specific abstractions, such as `JpaRepository` or `MongoRepository`. Those interfaces extend `CrudRepository` and expose the capabilities of the underlying persistence technology in addition to the rather generic persistence technology-agnostic interfaces such as `CrudRepository`. |
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.
Nit: We should clean up the superfluous whitespaces right after extend
.
src/main/asciidoc/repositories.adoc Outdated
==== | ||
| ||
Accessing the second page of `User` by a page size of 20 you could simply do something like this: | ||
To access the second page of `User` by a page size of 20, you could do something like the following: |
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.
Nit: We should clean up the superfluous whitespaces between could
and do
.
src/main/asciidoc/repositories.adoc Outdated
=== Web support | ||
| ||
NOTE: This section contains the documentation for the Spring Data web support as it is implemented as of Spring Data Commons in the 1.6 range. As it the newly introduced support changes quite a lot of things we kept the documentation of the former behavior in <<web.legacy>>. | ||
NOTE: This section contains the documentation for the Spring Data web support as it is implemented in the Spring Data Commons version (and later versions). As the newly introduced support changes many things, we kept the documentation of the former behavior in <<web.legacy>>. |
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.
How about …it is implemented in the current Spring Data Commons version.
?
src/main/asciidoc/repositories.adoc Outdated
==== Web Databinding Support | ||
| ||
Spring Data projections – generally described in <<projections>> – can be used to bind incoming request payloads by either using http://goessner.net/articles/JsonPath/[JSONPath] expressions (requires https://github.com/json-path/JsonPath[Jayway JasonPath] or https://www.w3.org/TR/xpath-31/[XPath] expressions (requires https://xmlbeam.org/[XmlBeam]). | ||
Spring Data projections (described in <<projections>>) can be used to bind incoming request payloads by either using http://goessner.net/articles/JsonPath/[JSONPath] expressions (requires https://github.com/json-path/JsonPath[Jayway JasonPath] or https://www.w3.org/TR/xpath-31/[XPath] expressions (requires https://xmlbeam.org/[XmlBeam]), as shown in the following example: |
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.
JasonPath -> JsonPath
src/main/asciidoc/repositories.adoc Outdated
| ||
[[web.legacy]] | ||
=== Legacy web support | ||
=== Legacy Web Support |
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 wonder whether we should remove this section as we have older version documentation available at docs.spring.io.
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.
Very much agreed. I removed it.
|`Optional<T>`|A Java 8 or Guava `Optional`. Expects the query method to return one result at most. In case no result is found `Optional.empty()`/`Optional.absent()` is returned. More than one result will trigger an `IncorrectResultSizeDataAccessException`. | ||
|`Option<T>`|An either Scala or JavaSlang `Option` type. Semantically same behavior as Java 8's `Optional` described above. | ||
|`Optional<T>`|A Java 8 or Guava `Optional`. Expects the query method to return one result at most. If no result is found, `Optional.empty()` or `Optional.absent()` is returned. More than one result triggers an `IncorrectResultSizeDataAccessException`. | ||
|`Option<T>`|Either a Scala or JavaSlang `Option` type. Semantically the same behavior as Java 8's `Optional`, described earlier. |
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.
Nit: Align JavaSlang to Javaslang (as mentioned in an earlier chapter)
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.
Yup. Thanks. (I checked, and it's Javaslang).
|`Page<T>`|A `Slice` with additional information, such as the total number of results. Requires a `Pageable` method parameter. | ||
|`GeoResult<T>`|A result entry with additional information, such as the distance to a reference location. | ||
|`GeoResults<T>`|A list of `GeoResult<T>` with additional information, such as the average distance to a reference location. | ||
|`GeoPage<T>`|A `Page` with `GeoResult<T>`, such as the average distance to a reference location. |
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.
Reminder for @mp911de: We should add RxJava 1/2 and Project Reactor types.
Awesome, thank you so much. I'll push a commit on your branch addressing the issues I found while going over the existing docs so we can have another review pass by someone else from the Data team. |
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 am not sure about the additions of these static resources in src/main/asciidoc
as it effectively means we have to replicate that into all other Spring Data modules.
I'd much more prefer to have them in https://github.com/spring-projects/spring-data-build/tree/master/resources so that the projects can refer to them through the build and we have a canonical place to make changes to all of the docs.
pom.xml Outdated
<groupId>org.springframework.data.build</groupId> | ||
<artifactId>spring-data-parent</artifactId> | ||
<version>2.1.0.BUILD-SNAPSHOT</version> | ||
<relativePath>../spring-data-build/parent/pom.xml</relativePath> |
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.
What's the reason we add this? We usually don't use relative paths to make sure the lookup of the parent is always done from the repository. Also, the declaration assumes that the parent project has been checked out into a folder of that exact name, which might not be correct. Also, if that folder has the project checked out in a different branch, it might see a different pom.
@@ -0,0 +1,51 @@ | |||
<script type="text/javascript" src="tocbot-3.0.2/tocbot.js"></script> |
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.
Should these additions (also docinfo.html
below) rather be done in the parent project and be consumed by the build? I'd want to avoid having to add these files to all Spring Data projects as that makes changes to them cumbersome.
src/main/asciidoc/docinfo.html Outdated
@@ -0,0 +1,15 @@ | |||
<style> |
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.
See the comment for docinfo-footer.html
.
Oliver Gierke; Thomas Darimont; Christoph Strobl; Mark Pollack; Thomas Risberg; Mark Paluch; Jay Bryant | ||
:revnumber: {version} | ||
:revdate: {localdate} | ||
:linkcss: |
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.
So far, I've preferred to have these settings in the build setup, as that means it can be managed centrally and doesn't have to be repeated in all index.adoc
for all Spring Data projects. Is there a reason to add them here?
I edited for spelling, punctuation, grammar, and corporate voice. I also added cross-references in a few places.
I added Tocbot and the files it requires (the tocbot-3.0.2 directory, docinfo.html, and docinfo-footer.html). I also moved spring.css into the stylesheets directory, to mirror our other projects. I also moved Asciidoctor attributes into index.adoc, so that command-line generation works correctly. Finally, I caught a couple of headings that weren't properly capitalized.
Mark proofread my first commit and caught a few things. I have fixed them. Thanks, Mark. :)
Mark Paluch and Oliver Gierke want to split the PR into a content portion and a ToC modernizing portion, so that we can do the ToC modernizing differently. To that end, I am removing all the ToC features from the PR.
I had switched to a different location for the stylesheet (to make this project look like the other Spring projects), but I then removed the stylesheet pursuant to changes requested by Oliver Gierke. That left the reference guide with no stylesheet. So I am putting back the former stylesheet arrangement.
Edited for spelling, punctuation, grammar, and corporate voice. Also added cross-references in a few places. Original pull request: #281.
Typo fixes, align Enable…Repositories annotation wording, document reactive return types. Original pull request: #281.
Edited for spelling, punctuation, grammar, and corporate voice. Also added cross-references in a few places. Original pull request: #281.
Typo fixes, align Enable…Repositories annotation wording, document reactive return types. Original pull request: #281.
Edited for spelling, punctuation, grammar, and corporate voice. Also added cross-references in a few places. Original pull request: #281.
Typo fixes, align Enable…Repositories annotation wording, document reactive return types. Original pull request: #281.
That's merged, polished, and backported now. |
I edited for spelling, punctuation, grammar, and corporate voice. I also added cross-references in a few places.