[GitHub] [camel] djencks commented on pull request #3860: CAMEL-14910: Bundling of the heavily distributed components

2020-05-30 Thread GitBox


djencks commented on pull request #3860:
URL: https://github.com/apache/camel/pull/3860#issuecomment-636393187


   I wrote https://github.com/apache/camel/pull/3876 which provides the 
functionality needed so your manual changes won't get overwritten by the java 
build.  I tried it on "ignite" in your branch and it appears to work properly.  
I'm not sure how to integrate all of this.  You might cherry pick my commit to 
your branch. You should then run the java build to get the updated component 
.adoc files and check that they look right and work with your gulp file change. 
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:
us...@infra.apache.org




[GitHub] [camel] djencks commented on pull request #3860: CAMEL-14910: Bundling of the heavily distributed components

2020-05-28 Thread GitBox


djencks commented on pull request #3860:
URL: https://github.com/apache/camel/pull/3860#issuecomment-635522339


   - You are still reading the file twice sometimes.  I was thinking of 
replacing
   ```
   if (summaryGroup != null || group != null) return true;
return false;
   ```
   with 
   ```
   return {summaryGroup, group}
   ```
   Then you can test `groupInfo.summaryGroup` and `groupInfo.group`.
   
   - Modifying the files in components/../src is better than  in 
docs/components/... but those files are partly generated as well, and I think 
the header attributes are currently  entirely generated, so your changes may 
get overwritten.  I think we can tweak the attribute generation to preserve 
specific attributes, including "group".  Otherwise we can find a way to 
generate this attribute as well.  The code is in 
tooling/maven/camel-package-maven-plugin/src/main/java/org/apache/camel/maven/packaging/UpdateReadmeMojo.java
 in the updateHeader method.  I think I see how to do this, so unless you are 
eager to work on this code I'll fix it in a separate PR. Let me know.
   
   - I think having the summary pages you mention will be a good idea, but 
there should probably be some description of the group, which I'm not qualified 
to write.  Let's add them and ask for someone to add some descriptive text. 
However, I think it would be better to add them in the PR for generating the 
index list on these pages.
   



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:
us...@infra.apache.org




[GitHub] [camel] djencks commented on pull request #3860: CAMEL-14910: Bundling of the heavily distributed components

2020-05-28 Thread GitBox


djencks commented on pull request #3860:
URL: https://github.com/apache/camel/pull/3860#issuecomment-635410111


   - I think for files that are not summary files, your code reads the file 
twice, first to determine it's not a summary page, and second to determine if 
it is in a group.  You could read the file once and apply both regex to 
determine both properties.
   
   - There needs to be a blank line after the header attributes before any 
content.  I think you removed this in some of the group files.
   
   - I think the changes to the group files are going to be overwritten when a 
java build is done.  We probably have to find a way to generate the group 
attribute from somewhere earlier.  I'll think about this.



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:
us...@infra.apache.org




[GitHub] [camel] djencks commented on pull request #3860: CAMEL-14910: Bundling of the heavily distributed components

2020-05-28 Thread GitBox


djencks commented on pull request #3860:
URL: https://github.com/apache/camel/pull/3860#issuecomment-635137587


   The code modifications look good, but I didn't try building it.  A couple of 
minor comments:
   
   - it might be worth reading each file only once and extracting both parent 
and child attributes at once, returning them in an object or array.
   
   - I'm not sure if deleting the AWS, AWS2 and google pages is a good idea.  
It might be better to turn them into real summary pages, at least with an index 
list.
   
   With this I think we'd be set for generating an index list on the summary 
pages.  Would you like to do that or would you prefer I do it?



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:
us...@infra.apache.org




[GitHub] [camel] djencks commented on pull request #3860: CAMEL-14910: Bundling of the heavily distributed components

2020-05-27 Thread GitBox


djencks commented on pull request #3860:
URL: https://github.com/apache/camel/pull/3860#issuecomment-634754864


   Yes. The files in docs/components should have an attribute indicating 
exactly where they come from, in components.



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:
us...@infra.apache.org




[GitHub] [camel] djencks commented on pull request #3860: CAMEL-14910: Bundling of the heavily distributed components

2020-05-27 Thread GitBox


djencks commented on pull request #3860:
URL: https://github.com/apache/camel/pull/3860#issuecomment-634480495


   That's the solution I was trying to suggest, except you've renamed the 
attributes I proposed:
   ```
   summary-group > parentGroup
   group > childGroup
   ```
   
   I think my names suit the situation marginally better, but don't have a 
strong preference.
   I'm not sure but think that it's more common to use dashes than camel case 
in asciidoc attribute names. Either will work.
   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:
us...@infra.apache.org




[GitHub] [camel] djencks commented on pull request #3860: CAMEL-14910: Bundling of the heavily distributed components

2020-05-26 Thread GitBox


djencks commented on pull request #3860:
URL: https://github.com/apache/camel/pull/3860#issuecomment-634127414







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:
us...@infra.apache.org