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

doc: active record and repository patterns #6923

Merged
merged 1 commit into from
Feb 26, 2020

Conversation

loicmathieu
Copy link
Contributor

@loicmathieu loicmathieu commented Jan 31, 2020

Fixes #6135
Fixes #6945

The following documentation changes are, for both MongoDB and Hibernate with Panache:

  • Rework the documentation to clearly present the two Panache flavours as equals in terms of support/implementation/...
  • Clearly name the two pattens : active records and repository
  • Add more repository documentation examples
  • Restructure the documentation

A PR on the quickstart will follows to duplicate the code to have exampes for both the active record and the repository patterns.

@loicmathieu
Copy link
Contributor Author

ping @gsmet, @FroMage, @emmanuelbernard can you give me feedback on this one ?

@loicmathieu
Copy link
Contributor Author

The quickstart PR with the repository example is here: quarkusio/quarkus-quickstarts#448

Copy link
Member

@emmanuelbernard emmanuelbernard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a full review. I just took a few areas and gave general advice to make the writing lighter and to the point.

docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
@loicmathieu
Copy link
Contributor Author

@emmanuelbernard all your feedbacks are on sentences that was previously existing and I move around, so I assume you're OK with the overal changes ;)
Anyway, I will update the documentation with your feedback as it makes sense :)

@loicmathieu loicmathieu force-pushed the feat/panache-entity-repository branch from 836f9d7 to a537db7 Compare February 3, 2020 15:47
@loicmathieu
Copy link
Contributor Author

@emmanuelbernard I updated the three sentences in both guides according to your feedback.

@emmanuelbernard
Copy link
Member

emmanuelbernard commented Feb 3, 2020 via email

@loicmathieu
Copy link
Contributor Author

@danielpetisme I added some part on the Hibernate with Panache documentation regarding the way to define Entities with the repository pattern. Feel free to give feedback.

@loicmathieu loicmathieu force-pushed the feat/panache-entity-repository branch from 7554454 to d311e32 Compare February 10, 2020 10:39
@loicmathieu loicmathieu marked this pull request as ready for review February 10, 2020 10:40
@loicmathieu
Copy link
Contributor Author

@gsmet @FroMage can I have your review on this documentation changes ?

Copy link
Member

@FroMage FroMage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@loicmathieu
Copy link
Contributor Author

@gsmet can you make a final review (you know my english ...) and merge this one ?

@geoand
Copy link
Contributor

geoand commented Feb 26, 2020

I'll check the english and let you know :)

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few small comments

docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/hibernate-orm-panache.adoc Outdated Show resolved Hide resolved
@@ -50,6 +50,9 @@ NOTE: the `list()` method might be surprising at first. It takes fragments of Pa
That makes for very concise but yet readable code.
MongoDB native queries are also supported.

NOTE: what you just see is the link:https://www.martinfowler.com/eaaCatalog/activeRecord.html[active record pattern], sometimes just called the entity pattern.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please just copy the changes I proposed in the other doc to this one :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, will do it

@geoand
Copy link
Contributor

geoand commented Feb 26, 2020

Don't forget to squash at the end of the process please :)

Fixes quarkusio#6135 and quarkusio#6945
WIP Apply suggestions from code review

will be squashed later

Co-Authored-By: Georgios Andrianakis <[email protected]>
@loicmathieu loicmathieu force-pushed the feat/panache-entity-repository branch from bc1ee4c to 737a697 Compare February 26, 2020 12:07
@loicmathieu
Copy link
Contributor Author

@geoand I integrate all your suggestions, copied them to the MongoDB with Panache guide and squashed everything.

Ready to be merged :)

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@geoand geoand added this to the 1.3.0 milestone Feb 26, 2020
@geoand geoand merged commit 8cc7747 into quarkusio:master Feb 26, 2020
@loicmathieu loicmathieu deleted the feat/panache-entity-repository branch February 26, 2020 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants