Skip to content
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

Ambiguity in Data Access chapter of online documentation #14642

Closed
gmazza opened this issue Sep 30, 2018 · 5 comments
Closed

Ambiguity in Data Access chapter of online documentation #14642

gmazza opened this issue Sep 30, 2018 · 5 comments
Labels
status: superseded An issue that has been superseded by another

Comments

@gmazza
Copy link

gmazza commented Sep 30, 2018

Website: https://docs.spring.io/spring-boot/docs/current/reference/html/howto-data-access.html#howto-use-two-entity-managers

GitHub page: https://github.com/spring-projects/spring-boot/blob/master/spring-boot-project/spring-boot-docs/src/main/asciidoc/howto.adoc

Current text:

80.8 Use Two EntityManagers
Even if the default EntityManagerFactory works fine, you need to define a new one. Otherwise, the presence of the second bean of that type switches off the default. To make it easy to do, you can use the convenient EntityManagerBuilder provided by Spring Boot. Alternatively, you can just the LocalContainerEntityManagerFactoryBean directly from Spring ORM, as shown in the following example:

Problem with above text:
1.) The title is not clear "Use Two EntityManagers", we're talking about EntityManagerFactories in this section and "two" seems tied to the specific example in this section but is not generalized for why someone might want to read this section, perhaps "Using Multiple EntityManagerFactories" would be clearer about the point of this section.

2.) This entry paragraph is not making it clear the target audience for it. The first sentence is written as if it holds for every one using Spring Data, period, because there is no filtering sentence allow the reader to know he can skip reading this section. If it said something like "Sometimes you may wish to work with multiple Entity Manager Factories because... In those cases, ..." it's much clearer to the reader that he may skip this section if it doesn't match his need.

3.) The first sentence is saying that we always need to "define" a new EntityManagerFactory. "Define" here is ambiguous over whether it means we need to override the default EMF or create a second EntityManagerFactory to work alongside it, nice to have it clarified.

4.) The second sentence is giving a non-sensical reason for why we need to create this second EntityManagerFactory (whether sibling or override), namely that the second's existence would otherwise shut off the default EMF which is working fine for us per the first sentence. But if we didn't create this second EMF to begin with, how could it shut off the default EMF?

In other words, the second sentence is like saying "If you want to check box A, you must also check box B, because checking box B negates the checking of box A" (something that we don't want given that we decided to check box A to begin with.)

5.) The last sentence "Alternatively, you can just ??? the LCEMFB..." is missing a verb at ???, I'm not sure if you're trying to say "override" or "use" or...? The missing verb further adds confusion about what we're trying to accomplish in this section.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Sep 30, 2018
@philwebb philwebb added type: documentation A documentation update status: ideal-for-contribution An issue that a contributor can help us with and removed status: waiting-for-triage An issue we've not yet triaged labels Oct 1, 2018
@philwebb philwebb modified the milestones: 2.x, General Backlog Oct 1, 2018
@nicce
Copy link

nicce commented Oct 22, 2018

Is it ok to take a look at this one? Haven't contribute to Open Source before, would really like to start.

@philwebb
Copy link
Member

@nicce Absolutely! The reference documentation that gets published with each release is built from the source in the spring-boot-project/spring-boot-docs/src/main/asciidoc folder. This particular section is in howto.adoc.

The documentation is written in Asciidoctor markup. If you've not used it before this quick syntax guide is very helpful. I use the chrome plugin usually to check the file locally as I'm editing it. There's also a nice plugin for Atom.

You can make the edits suggested in the issue and then submit a pull-request. If you have any issues, just comment here and we'll do our best to help.

Thanks for getting involved!

@nicce
Copy link

nicce commented Oct 23, 2018

I'm having a hard time building the project using ./mvnw clean install. Test failing in the AutoConfigure module even in the master branch. Thought that it was me failing to set up the project correctly in IntelliJ but fails on the spring ci server as well. Are there any known issue on this? Or are you supposed to just try and fix it?

@philwebb
Copy link
Member

philwebb commented Oct 23, 2018

@nicce Unfortunately the build was broken for a little while yesterday. It's currently green so it should build cleanly if you run ./mnvw clean install -U. Since it's just a documentation change you can just push whatever you have and we'll check it when we merge.

EDIT: I see you've submitted the PR already. Thanks!

@philwebb
Copy link
Member

Closing in favor of PR #14928

@philwebb philwebb added status: superseded An issue that has been superseded by another and removed status: ideal-for-contribution An issue that a contributor can help us with type: documentation A documentation update labels Oct 23, 2018
@philwebb philwebb removed this from the General Backlog milestone Oct 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: superseded An issue that has been superseded by another
Projects
None yet
Development

No branches or pull requests

4 participants