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.

Sign up for GitHub

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

Jump to bottom

Helidon Reactive documentation #1483

Merged
merged 11 commits into from
Apr 22, 2020
Merged

Helidon Reactive documentation #1483

merged 11 commits into from
Apr 22, 2020

Conversation

danielkec
Copy link
Contributor

@danielkec danielkec commented Mar 10, 2020 •
edited
Loading

related PR helidon-io/helidon-build-tools/pull/90

Signed-off-by: Daniel Kec daniel.kec@oracle.com

@danielkec danielkec self-assigned this Mar 10, 2020
@danielkec danielkec added the reactive Reactive streams and related components label Mar 10, 2020
@danielkec danielkec added this to the 2.0.0 milestone Mar 10, 2020
@m0mus m0mus mentioned this pull request Mar 11, 2020
Closed
39 tasks
@danielkec danielkec changed the title WIP: Helidon Reactive Messaging doc Helidon Reactive documentation Mar 26, 2020
@danielkec danielkec requested a review from m0mus March 26, 2020 15:58
danielkec
danielkec commented Mar 26, 2020
View reviewed changes
docs/sitegen.yaml Outdated Show resolved Hide resolved
@danielkec danielkec added the docs label Mar 26, 2020
@danielkec danielkec requested a review from ljamen March 27, 2020 10:07
ljamen
ljamen reviewed Mar 27, 2020
View reviewed changes
Copy link
Contributor

@ljamen ljamen left a comment

Choose a reason for hiding this comment

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

This looks good. We might need to follow up with D on how to address repeated content.

docs/sitegen.yaml Outdated Show resolved Hide resolved
docs/mp/messaging/02_message.adoc Outdated Show resolved Hide resolved
docs/mp/messaging/01_reactive-messaging.adoc Outdated Show resolved Hide resolved
docs/mp/messaging/01_reactive-messaging.adoc Outdated Show resolved Hide resolved
ljamen
ljamen previously approved these changes Mar 27, 2020
View reviewed changes
Copy link
Contributor

@ljamen ljamen left a comment

Choose a reason for hiding this comment

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

Changes look good, request was made to use includes for repeated topics.

@danielkec
Helidon Reactive documentation
ff5e328 
  * Message wrapper and ack
  * Operators and Connectors

Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@danielkec
Better rephrasing
571d34f 
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@danielkec
Include instead of includes
45a9bd7 
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@danielkec
Multi/Single doc
0edf176 
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@danielkec
Copy link
Contributor Author

@m0mus Rebased on top of latest doc changes, added Multi/Single doc

@m0mus
Copy link
Contributor

m0mus commented Mar 30, 2020

@danielkec @ljamen Repeating my question to both of you. I posted it as one of the comments, but after the last commit it got collapsed and not seen anymore. So, I'm not sure that you've seen it.
Would it be better to put shared pieces in shared folder and reference them from se and mp instead of referencing se parts from mp? In this case it will be easy to identify what is shared.

@danielkec
Copy link
Contributor Author

@danielkec @ljamen Repeating my question to both of you. I posted it as one of the comments, but after the last commit it got collapsed and not seen anymore. So, I'm not sure that you've seen it.
Would it be better to put shared pieces in shared folder and reference them from se and mp instead of referencing se parts from mp? In this case it will be easy to identify what is shared.

I like share folder approach

akarnokd
akarnokd reviewed Mar 30, 2020
View reviewed changes
Copy link
Collaborator

@akarnokd akarnokd left a comment

Choose a reason for hiding this comment

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

Suggested some changes to the operator titles.

docs/se/reactivestreams/02_engine.adoc Outdated Show resolved Hide resolved
docs/se/reactivestreams/02_engine.adoc Outdated Show resolved Hide resolved
docs/se/reactivestreams/02_engine.adoc Outdated Show resolved Hide resolved
docs/se/reactivestreams/02_engine.adoc Outdated Show resolved Hide resolved
docs/se/reactivestreams/02_engine.adoc Outdated Show resolved Hide resolved
docs/se/reactivestreams/02_engine.adoc Outdated Show resolved Hide resolved
docs/se/reactivestreams/02_engine.adoc Outdated Show resolved Hide resolved
danielkec added 3 commits April 1, 2020 14:16
@danielkec
Shared folder for common stuff
e6394cd 
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@danielkec
Discrepancies pointed out by @akarnokd
72ff3aa 
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@danielkec
Merge remote-tracking branch 'upstream/master' into mp-messaging-doc
f407948
danielkec
danielkec commented Apr 1, 2020
View reviewed changes
docs/se/reactivestreams/01_introduction.adoc Outdated
Comment on lines 19 to 22
= Overview
:toc:
:toc-placement: preamble
:description: Reactive Streams Operators support in Helidon SE
:parentTitle: Se & Mp
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@m0mus Parent title override will kick in when helidon-io/helidon-build-tools/pull/92 is used for site creation

@danielkec
Copy link
Contributor Author

@akarnokd @ljamen Lists of operators are little stark now, I am planning to expand operators documentation in future PR's with fancy marble diagrams. Would like to just lay the ground structure in this PR so it wouldn't grow humongous and unreviewable.

@danielkec danielkec requested a review from ljamen April 1, 2020 12:42
@danielkec
Copy link
Contributor Author

Continuation of helidon-io/helidon-build-tools#92 (comment)
@romain-grecourt

The title in your screenshot is "Se & Mp". I thought you are now using distinct documents and including shared content using the includes directive.

Yes I do, thats the parentTitle override helidon-io/helidon-build-tools/pull/92 is about

If they are separate document, there is one for se and one for mp right ? If so then why is the title "Se & Mp" ?

I am forced to use separate documents by technical reasons, but its functionality available in both flawors of Helidon, so using parentTitle to let user know.

Also, the route is 01_introduction but the 2nd part of the title is "Overview", that's not consistent.

My bad should be 01_overview

ljamen
ljamen reviewed Apr 8, 2020
View reviewed changes
docs/mp/reactivestreams/01_introduction.adoc Outdated Show resolved Hide resolved
docs/shared/reactivestreams/03_rsoperators.adoc Outdated Show resolved Hide resolved
docs/shared/reactivestreams/02_engine.adoc Outdated Show resolved Hide resolved
@m0mus m0mus mentioned this pull request Apr 8, 2020
Closed
@m0mus m0mus added the P1 label Apr 9, 2020
@danielkec
Function compatible mapper
f673ecd 
Signed-off-by: Daniel Kec <daniel.kec@oracle.com>
@danielkec danielkec added the messaging Reactive Messaging label Apr 14, 2020
@ljamen ljamen mentioned this pull request Apr 16, 2020
Closed
46 tasks
@paulparkinson
Copy link
Contributor

We still need to add the SE messaging examples or I am missing them? Otherwise looks good so far.

m0mus
m0mus approved these changes Apr 22, 2020
View reviewed changes
Copy link
Contributor

@m0mus m0mus left a comment

Choose a reason for hiding this comment

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

LGTM

@danielkec danielkec merged commit 355f646 into helidon-io:master Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ljamen ljamen ljamen left review comments

@romain-grecourt romain-grecourt romain-grecourt left review comments

@akarnokd akarnokd akarnokd left review comments

@m0mus m0mus m0mus approved these changes

Assignees

@danielkec danielkec

Labels
docs messaging Reactive Messaging P1 reactive Reactive streams and related components
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@danielkec @m0mus @paulparkinson @akarnokd @romain-grecourt @ljamen