-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
My second application tutorial #34401
My second application tutorial #34401
Conversation
317f349
to
a0085b5
Compare
this looks good to me! +1 on adding it asap. @cescoffier i know you were encouraging we got something similar to this as part of getting started. |
It's on my review list - will squeeze it tomorrow |
a0085b5
to
c915ebe
Compare
🙈 The PR is closed and the preview is expired. |
git clone {quickstarts-clone-url} | ||
---- | ||
|
||
The solution is located in the `getting-started-dev-services` {quickstarts-tree-url}/getting-started-dev-services[directory]. |
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.
Why 'dev-service', shouldn't it be 'crud' ?
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.
or 'rest'
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 don't think it could be 'rest', because https://github.com/quarkusio/quarkus-quickstarts/tree/main/getting-started is a rest application. It could be 'crud' ... to me, dev services was the big gap that we didn't have content for, so I named it that way. But you're right that crud is the characteristic feature of the application code, and we do already have https://github.com/quarkusio/quarkus-quickstarts/tree/main/getting-started-reactive-crud/. So I'll rename it that way to be consistent.
There's also overlap between the app and https://github.com/quarkusio/quarkus-quickstarts/tree/main/hibernate-orm-panache-quickstart, but the 'crud' name doesn't make the overlap worse, just more obvious. :) The key difference is that one is designed to be coded by a user and the other is intended to be downloaded on app creation, so I think it's right that they stay separate.
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.
Actually, I think I've changed my mind again. getting-started-dev-services
seems like the right file name for this article, and then people would expect that the code in the other repo would have the same name as the article.
Now, try stopping and starting your application, and then visiting http://localhost:8080/hello/names. | ||
You'll see that Alice and Bob are always included in the list of names. | ||
|
||
== Controlling dev services |
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 wondering if the rest of the document should be in this guide or a more "reference" guide.
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.
To me, it feels like we're leaving people with an incomplete picture if we don't include it; we've told them about dev services, but not giving them any hints about what happens if they don't want to use dev services. They might think they can't use a real database with Quarkus, and I'm not sure we want them to have to go read the both configuration reference and the dev services reference to figure out that it's possible. "Use profiles to conditionally enable dev services" would span the two references, which is an extra barrier to people figuring out the idea on their own.
Just a note that I would like to have a look at this before we merge. I’ll have a look next Tuesday when I’m back from Riviera DEV. |
Thanks for the review, @cescoffier! Updates pushed. |
748abc1
to
29ab089
Compare
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 for that. I spotted a few things. Some are minor, some not so much, please have a look :).
|
||
This tutorial shows you how to create an application which writes to and reads from a database. | ||
You will use Dev Services, so you will not actually download, configure, or even start the database yourself. | ||
You will use Panache to make reading and writing data easier. |
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 have the Logging with Panache
too so I wonder if we should be more specific? But it's just a proposal, the current wording somehow works for me.
You will use Panache to make reading and writing data easier. | |
You will use Hibernate ORM with Panache to make reading and writing data easier. |
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.
"Logging with Panache" reminds me I meant to ask about that bit of the docs. Done now.. Perhaps some of this discussion goes on that PR.
I'm in two minds about your proposal
Keep as is
- We want to walk people through what's new and different in Quarkus, so should focus on the new bit, ie Panache
- "Hibernate ORM with Panache" is a bit of a mouthful
Revise
- Hibernate is familiar to people and helps anchor their understand
What about "You will use Panache, a layer on top of Hibernate ORM, to make reading and writing data easier.'? That's less consistent with our 'x with Panache' convention, but more consistent with how external people refer to Panache.
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.
Well, we have a convention, we should follow it.
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.
That being said You will use Panache, a layer on top of Hibernate ORM, to make reading and writing data easier
is not bad. But really, if everyone has their own version of it, it's going to be a mess.
7333f12
to
e8f57c2
Compare
Thanks, @gsmet! I've committed all the small suggestions and implemented the bigger ones, except the one or two still open, about naming things. |
Oh, whoops, I'd missed 25 suggestions that the GitHub UI was hiding from me. Ok, now I think I've got them 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 spotted a micro issue in the Windows note.
Also this needs a big squash.
|
||
This tutorial shows you how to create an application which writes to and reads from a database. | ||
You will use Dev Services, so you will not actually download, configure, or even start the database yourself. | ||
You will use Panache to make reading and writing data easier. |
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.
That being said You will use Panache, a layer on top of Hibernate ORM, to make reading and writing data easier
is not bad. But really, if everyone has their own version of it, it's going to be a mess.
ca4e532
to
9a92ddb
Compare
Squashed to include review comments. Co-Authored-By: Guillaume Smet <guillaume.smet@gmail.com>
9a92ddb
to
774b436
Compare
Thanks! |
Resolves #31618
The companion code lives in quarkusio/quarkus-quickstarts#1293.
I'll mark this as a draft and work with the docs team.