[GitHub] [camel] swapy opened a new pull request #5311: Add proper endChoice() and end() blocks

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] swapy opened a new pull request #5311: Add proper endChoice() and end() blocks

GitBox

swapy opened a new pull request #5311:
URL: https://github.com/apache/camel/pull/5311


   Need to add proper endChoice() and end() blocks to show correct way to work with choices(as a best practice).
   
   Since this is quite fundamental requirement, it is necessary to have complete documentation stating all facts.
   
   Also not sure about following:
   
   1. Do we need to add endChoice() after otherwise()?
   2. Do we need to make any changes in xml based configuration or it ends by default?(I suppose it works by default)?
   3. Do we also need to document meaning of endChoice() and end()?
   4. Additionally, should we mention limitation on loop nesting in documentation, an example would also add value.
   


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] davsclaus commented on a change in pull request #5311: Add proper endChoice() and end() blocks

GitBox

davsclaus commented on a change in pull request #5311:
URL: https://github.com/apache/camel/pull/5311#discussion_r611010767



##########
File path: core/camel-core-engine/src/main/docs/modules/eips/pages/choice-eip.adoc
##########
@@ -40,14 +40,49 @@ RouteBuilder builder = new RouteBuilder() {
             .choice()
                 .when(simple("${header.foo} == 'bar'"))
                     .to("direct:b")
+                .endChoice()
                 .when(simple("${header.foo} == 'cheese'"))
                     .to("direct:c")
+                .endChoice()

Review comment:
       Can you restore this example back to what it was before - this is 99% of the normal use-cases.
   
   And then below it would be good to add a header section about endChoice and then you can use the multicast example as its example.




--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] swapy commented on a change in pull request #5311: Add proper endChoice() and end() blocks

GitBox
In reply to this post by GitBox

swapy commented on a change in pull request #5311:
URL: https://github.com/apache/camel/pull/5311#discussion_r611013143



##########
File path: core/camel-core-engine/src/main/docs/modules/eips/pages/choice-eip.adoc
##########
@@ -40,14 +40,49 @@ RouteBuilder builder = new RouteBuilder() {
             .choice()
                 .when(simple("${header.foo} == 'bar'"))
                     .to("direct:b")
+                .endChoice()
                 .when(simple("${header.foo} == 'cheese'"))
                     .to("direct:c")
+                .endChoice()

Review comment:
       Hi Dave,
   Sure Dave, I have made changes. Let me know if they are correct. I am happy to make any additional changes that you can share. :+1:
   
   I have done some minor reordering of documentation, let me know if that's okay. (TIP is moved from being in between dsl and xml to end of doc)
   




--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] davsclaus commented on a change in pull request #5311: Add proper endChoice() and end() blocks

GitBox
In reply to this post by GitBox

davsclaus commented on a change in pull request #5311:
URL: https://github.com/apache/camel/pull/5311#discussion_r611014944



##########
File path: core/camel-core-engine/src/main/docs/modules/eips/pages/choice-eip.adoc
##########
@@ -40,14 +40,49 @@ RouteBuilder builder = new RouteBuilder() {
             .choice()
                 .when(simple("${header.foo} == 'bar'"))
                     .to("direct:b")
+                .endChoice()
                 .when(simple("${header.foo} == 'cheese'"))
                     .to("direct:c")
+                .endChoice()

Review comment:
       Thanks for your patience and helping with the project. You are much welcome to keep contributing, thanks.




--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel] davsclaus merged pull request #5311: Add proper endChoice() and end() blocks

GitBox
In reply to this post by GitBox

davsclaus merged pull request #5311:
URL: https://github.com/apache/camel/pull/5311


   


--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]