Reviewing Datasource guide #32802

MichalMaler · 2023-04-20T13:26:42Z

I am proposing a complete review of the Datasource guide, which act as the first iteration of the refactorization.
In the next iterations, the Datasource content will be split into 3 Diataxes module templates.

github-actions · 2023-04-20T14:21:23Z

🙈 The PR is closed and the preview is expired.

yrodiere

Thanks, here are my two cents.

I'm afraid a lot of it feels like nitpicking over the form rather than the content, but I guess that cannot be avoided if the goal is to improve the form :)

docs/src/main/asciidoc/datasource.adoc

MichalMaler · 2023-04-24T10:07:51Z

@yrodiere I patched the For more information sentences and drop the confusing lines on 610-611. Patched and rebased.

Update JDBC table and create another one (for Reactive drivers). Will move that into the new Reference section when all I did will be approved.

docs/src/main/asciidoc/datasource.adoc

MichalMaler · 2023-04-28T14:56:06Z

@yrodiere Hello Yoann! We did the lion's share of work here! Applied all of your's and @jmartisk suggestions.

MichalMaler · 2023-05-04T13:33:11Z

Hello @michelle-purcell ! Please, I applied feedback from my SMEs, and already did a first iteration of general review. Could I ask you for a little bit of your magic? :)

docs/src/main/asciidoc/datasource.adoc

michelle-purcell

@MichalMaler - A super start to recomposing 🦾 LGTM just some minor comments for minimalism and for Quarkus style consistency. See my comments above. Thanks :-)

docs/src/main/asciidoc/datasource.adoc

Sanne

I've submitted some minor nitpicks, things I suspect I'd find confusing as a newcomer.

I hope they can be addressed, but feel free to dismiss them if it would take too long or of it's not a good time - I would prefer iterations over blocking this for too long.

docs/src/main/asciidoc/datasource.adoc

Sanne · 2023-05-19T11:14:07Z

docs/src/main/asciidoc/datasource.adoc

+
+When using one of the built-in datasource kinds, the JDBC and Reactive drivers are resolved automatically to match the values from these tables.
+
+.Database kind to JDBC driver mapping


Let's rework the title as "Database platform kind to JDBC driver mapping" ?
So it better expressed what we mean by kind, and make it findable too

michelle-purcell · 2023-05-19T12:43:25Z

docs/src/main/asciidoc/datasource.adoc

@@ -3,62 +3,61 @@ This guide is maintained in the main Quarkus repository
 and pull requests should be submitted there:
 https://github.com/quarkusio/quarkus/tree/main/docs/src/main/asciidoc
 ////
-= Datasources
+[id="datasources"]
+= Configure datasources in Quarkus


@MichalMaler /@Sanne - The Vale linter spelling warning on the term datasources is creating noise in this PR.

As this is an accepted term in Quarkus, I think we should add an exception to this rule. WDYT?

Yeah, I agree :)

Well. This is the discussion. We confuse "data source" (noun) with "datasource" (the code artifact.

Technically speaking, the title should be "Configure data sources in Quarkus". Depends on how semantically correct we want to be. Quarkus has habitually used the two forms interchangably, but that doesn't mean it is correct. ;)

(the id can be datasources, or should be datasource if we want to stick with file conventions... ).

MichalMaler · 2023-05-19T14:15:34Z

@Sanne Thank you so much, Sanne for your review! Applied some of your comments and will continue with the rest first thing on Monday.

Signed-off-by: Michal Maléř <[email protected]> Applying suggestions form an SME review-1 Signed-off-by: Michal Maléř <[email protected]> Applying suggestions form an SME review-2 Signed-off-by: Michal Maléř <[email protected]> Applying suggestions form an SME review-3 Signed-off-by: Michal Maléř <[email protected]> Apply suggestions from code review Co-authored-by: Yoann Rodière <[email protected]> Tables revamped Signed-off-by: Michal Maléř <[email protected]> FIxes Signed-off-by: Michal Maléř <[email protected]> Fixes Signed-off-by: Michal Maléř <[email protected]> Fixes Signed-off-by: Michal Maléř <[email protected]> Fixes Signed-off-by: Michal Maléř <[email protected]> Fixes Signed-off-by: Michal Maléř <[email protected]> Fixes Signed-off-by: Michal Maléř <[email protected]> Fixes Signed-off-by: Michal Maléř <[email protected]> Fixes Signed-off-by: Michal Maléř <[email protected]> Apply suggestions from code review Co-authored-by: Michelle Purcell <[email protected]> Erin fixes1 Signed-off-by: Michal Maléř <[email protected]> Erin fixes2 Signed-off-by: Michal Maléř <[email protected]> Fixes Signed-off-by: Michal Maléř <[email protected]> Fixes2 Signed-off-by: Michal Maléř <[email protected]> Fixes2 Signed-off-by: Michal Maléř <[email protected]>

MichalMaler · 2023-05-24T11:01:14Z

Hello everyone!

This guide has received a humongous review and underwent numerous updates, incorporating suggestions from @jmartisk , @yrodiere , @Sanne , @michelle-purcell , and @ebullient . I propose merging this iteration and scheduling a meeting with someone knowledgeable about Datasource to discuss the next iteration. Taking a more team-oriented approach will allow us to further enhance the content with professional input. Without additional support from you, seasoned developers, there is limited progress I can make on this content that needs to be published in RHBOQ too. Let's move forward together! :)

Regarding the future rework to Diataxes templates, would you @ebullient help me with some planning? I would like to have your experienced opinion before taking this further too.

Sanne · 2023-05-24T11:21:12Z

@MichalMaler agreed! Better to have multiple iterations. Merging...

Please remember there were some pending suggestions (which we seem we had already agreed on?) which haven't been applied yet.

MichalMaler · 2023-05-24T11:40:40Z

@Sanne
Oh, I found only one of your suggestions that I prepared but forgot to add to the commit. It was a simplification of the following explenation:

<1> If you only have a single Reactive driver extension or are running tests and only have a single test-scoped Reactive driver extension added, then this is optional.

Proposing:
<1> This configuration value is only required if there is more than one Reactive driver extension on the classpath.

The rest was applied or tested with @jmartisk and found correct.

Or is there anything else overlooked? I was closing the suggestions as I applied them to a commit.

Sanne · 2023-05-24T15:28:44Z

@MichalMaler np, it gets confusing when it's this large.

There's these pending:

Reviewing Datasource guide #32802 (comment)
and Erin's points about datasource vs Datasource vs data source (I don't know..)

And +1 for your proposal, just send other PRs so we can resume nitpicking :)

MichalMaler · 2023-05-24T16:03:38Z

@MichalMaler np, it gets confusing when it's this large.

There's these pending:

Reviewing Datasource guide #32802 (comment)

and Erin's points about datasource vs Datasource vs data source (I don't know..)

And +1 for your proposal, just send other PRs so we can resume nitpicking :)

Oh, they were both updated :) The suggestion you adressed was a comment, not a suggestion, so I could not close/solve it when done :) And I addressed Erin's proposal to change the main heading too :) It says Data source now.

Next smaller PR coming soon :) @Sanne

jglick · 2024-06-10T16:56:01Z

docs/src/main/asciidoc/datasource.adoc

+* Derby cannot be embedded into the application in native mode, and only remote connection is supported.
+[IMPORTANT]
+====
+Embedding H2 within your native image is not recommended. 


Is there any more detail on why this is not recommended? See #6320 (comment)

As far as I know, just because of uncertainty regarding bugs, feature coverage, performance. It started out as something experimental and didn't change much since then.

We haven't had many bug reports either, so you can interpret that any way you like: either nobody uses H2 in native images, or it works fine.

MichalMaler requested a review from gsmet April 20, 2023 13:26

quarkus-bot bot added the area/documentation label Apr 20, 2023

MichalMaler requested review from jmartisk, yrodiere and ebullient April 20, 2023 13:27

ebullient requested a review from Sanne April 21, 2023 16:49

yrodiere reviewed Apr 24, 2023

View reviewed changes

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch 2 times, most recently from 9930174 to 8527799 Compare April 24, 2023 10:06

jmartisk reviewed Apr 24, 2023

View reviewed changes

docs/src/main/asciidoc/datasource.adoc Outdated Show resolved Hide resolved

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch from 8527799 to 5df4667 Compare April 24, 2023 13:31

MichalMaler requested a review from jmartisk April 25, 2023 06:10

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch from 80f322a to e8c0121 Compare April 28, 2023 14:41

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch 3 times, most recently from fe8ad63 to dda9559 Compare May 2, 2023 12:34

MichalMaler requested a review from michelle-purcell May 2, 2023 13:13

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch 3 times, most recently from 30530c2 to 07b1049 Compare May 4, 2023 12:57

michelle-purcell reviewed May 4, 2023

View reviewed changes

docs/src/main/asciidoc/datasource.adoc Outdated Show resolved Hide resolved

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch from 07b1049 to efd267e Compare May 4, 2023 14:58

michelle-purcell approved these changes May 5, 2023

View reviewed changes

michelle-purcell reviewed May 5, 2023

View reviewed changes

docs/src/main/asciidoc/datasource.adoc Outdated Show resolved Hide resolved

MichalMaler commented May 5, 2023

View reviewed changes

docs/src/main/asciidoc/datasource.adoc Outdated Show resolved Hide resolved

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch 3 times, most recently from f956a53 to 91f68f1 Compare May 5, 2023 14:05

gastaldi reviewed May 5, 2023

View reviewed changes

docs/src/main/asciidoc/datasource.adoc Outdated Show resolved Hide resolved

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch from 91f68f1 to 71b38dc Compare May 15, 2023 11:56

jmartisk approved these changes May 16, 2023

View reviewed changes

michelle-purcell reviewed May 19, 2023

View reviewed changes

docs/src/main/asciidoc/datasource.adoc Outdated Show resolved Hide resolved

Sanne requested changes May 19, 2023

View reviewed changes

michelle-purcell reviewed May 19, 2023

View reviewed changes

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch 2 times, most recently from 42aa705 to 2cdc106 Compare May 23, 2023 13:01

MichalMaler force-pushed the QDOCS-197-DataSource-refactor branch from 2cdc106 to 8f3ae4d Compare May 24, 2023 10:29

MichalMaler requested a review from Sanne May 24, 2023 11:04

Sanne approved these changes May 24, 2023

View reviewed changes

Sanne merged commit 0a42ca8 into quarkusio:main May 24, 2023

quarkus-bot bot added this to the 3.2 - main milestone May 24, 2023

MichalMaler deleted the QDOCS-197-DataSource-refactor branch May 24, 2023 12:10

MichalMaler mentioned this pull request May 26, 2023

Improving the Datasource guide #33634

Merged

jglick reviewed Jun 10, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reviewing Datasource guide #32802

Reviewing Datasource guide #32802

MichalMaler commented Apr 20, 2023

github-actions bot commented Apr 20, 2023 •

edited

Loading

yrodiere left a comment

MichalMaler commented Apr 24, 2023 •

edited

Loading

MichalMaler commented Apr 28, 2023 •

edited

Loading

MichalMaler commented May 4, 2023

michelle-purcell left a comment

Sanne left a comment

Sanne May 19, 2023

michelle-purcell May 19, 2023 •

edited

Loading

MichalMaler May 19, 2023

ebullient May 22, 2023

MichalMaler commented May 19, 2023

MichalMaler commented May 24, 2023 •

edited

Loading

Sanne commented May 24, 2023

MichalMaler commented May 24, 2023

Sanne commented May 24, 2023

MichalMaler commented May 24, 2023

jglick Jun 10, 2024

yrodiere Jun 11, 2024


		When using one of the built-in datasource kinds, the JDBC and Reactive drivers are resolved automatically to match the values from these tables.

		.Database kind to JDBC driver mapping

Reviewing Datasource guide #32802

Reviewing Datasource guide #32802

Conversation

MichalMaler commented Apr 20, 2023

github-actions bot commented Apr 20, 2023 • edited Loading

yrodiere left a comment

Choose a reason for hiding this comment

MichalMaler commented Apr 24, 2023 • edited Loading

MichalMaler commented Apr 28, 2023 • edited Loading

MichalMaler commented May 4, 2023

michelle-purcell left a comment

Choose a reason for hiding this comment

Sanne left a comment

Choose a reason for hiding this comment

Sanne May 19, 2023

Choose a reason for hiding this comment

michelle-purcell May 19, 2023 • edited Loading

Choose a reason for hiding this comment

MichalMaler May 19, 2023

Choose a reason for hiding this comment

ebullient May 22, 2023

Choose a reason for hiding this comment

MichalMaler commented May 19, 2023

MichalMaler commented May 24, 2023 • edited Loading

Sanne commented May 24, 2023

MichalMaler commented May 24, 2023

Sanne commented May 24, 2023

MichalMaler commented May 24, 2023

jglick Jun 10, 2024

Choose a reason for hiding this comment

yrodiere Jun 11, 2024

Choose a reason for hiding this comment

github-actions bot commented Apr 20, 2023 •

edited

Loading

MichalMaler commented Apr 24, 2023 •

edited

Loading

MichalMaler commented Apr 28, 2023 •

edited

Loading

michelle-purcell May 19, 2023 •

edited

Loading

MichalMaler commented May 24, 2023 •

edited

Loading