Skip to content

CAMEL-14910: Bundling of the heavily distributed components#3860

Closed
AemieJ wants to merge 0 commit intoapache:masterfrom
AemieJ:CAMEL-14910
Closed

CAMEL-14910: Bundling of the heavily distributed components#3860
AemieJ wants to merge 0 commit intoapache:masterfrom
AemieJ:CAMEL-14910

Conversation

@AemieJ
Copy link
Contributor

@AemieJ AemieJ commented May 26, 2020

  • Particular queries were raised in this issue.
  1. Should there be summary pages for groups of related components?
    There already existed summary pages for groups including Ignite, Spring, and a few others. However, as this brings in clarification to understand the documentation, I also added a summary for other groups including AWS, AWS 2, Google.

  2. What should their structure and appearance be?
    In the nav panel, I believe that the highly distributed related components should be bundled and grouped together within the parent component.

  3. Should the nav pane indent the components in a group?
    As for user manual documentation under camel Kafka and others, the nav pane is well indented with subgroups and parent groups. Thus, it has been done in the same manner for this PR.

Copy link
Contributor

@oscerd oscerd left a comment

Choose a reason for hiding this comment

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

As reported in this file:

// this file is auto generated and changes to it will be overwritten
// make edits in docs/*nav.adoc.template files instead

The list of components is autogenerated, so this update will be lost at the first rebuild.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

As reported in this file:

// this file is auto generated and changes to it will be overwritten
// make edits in docs/*nav.adoc.template files instead

The list of components is autogenerated, so this update will be lost at the first rebuild.

Sir, I made the changes in the docs/components/modules/ROOT/nav.adoc file itself to nest the related components together.

@oscerd
Copy link
Contributor

oscerd commented May 26, 2020

Yes, but since this file is autogenerated on each camel build, your update will be lost at first rebuild. So actually this cannot be merged.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

Yes, but since this file is autogenerated on each camel build, your update will be lost at a first rebuild. So actually this cannot be merged.

Alright understood, but you specified edits could be done on nav.adoc.template files, however, I am unable to find them on the repo, I can only find all the existing nav.adoc files.

@oscerd
Copy link
Contributor

oscerd commented May 26, 2020

That's the comment reported there. Grouping the aws2, google etc component, requires to modify the way the nav.adoc is generated.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

That's the comment reported there. Grouping the aws2, google etc component, requires to modify the way the nav.adoc is generated.

I edited the .adoc.template file related to the component nav.

@omarsmak
Copy link
Member

Is this groping happened manually? I mean how did we determine these are related components? If yes, wouldn't makes sense to generate these data programmatically? I think at some point we will lose track of maintaining such list :)

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

@omarsmak They have the initial starting as in for related components, let's say ignite all the naming is done as per ignite- component name. This is how the relativeness was determined based on the naming of the files.

Additionally, even the distribution for the user panel nav template has been done in the similar manner if taken a look at.

@omarsmak
Copy link
Member

@AemieJ is great to hear that, my question was actually, does already generate the groupings programmatically or you have to create the grouping templates beforehand?

@djencks
Copy link
Contributor

djencks commented May 26, 2020

This patch doesn't work. The template does not need to be changed, only the generation code in gulpfile.js.

It's unfortunate that jira is so loosely connected to GitHub. On the Jira issue I suggested a "depth" attribute in the page to determine the nesting depth. I don't see a way to generate this automatically and reliably. Any other ideas?

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

This patch doesn't work. The template does not need to be changed, only the generation code in gulpfile.js.

It's unfortunate that jira is so loosely connected to GitHub. On the Jira issue I suggested a "depth" attribute in the page to determine the nesting depth. I don't see a way to generate this automatically and reliably. Any other ideas?

I didn't see the reply on Jira, sorry about that, I provided an idea to nest them using depth on Jira. I believe it's best to include a summary for all the relative components, some of which I have written. I will revert back the template.adoc.

@djencks
Copy link
Contributor

djencks commented May 26, 2020

Well, I didn't notice the jira issue is closed:-) Lets discuss this here.

I think your idea of computing using the names will work. However, I would prefer that the information used to structure the generated lists be available as attribute data. I think this will make it easier to understand what is happening and more useful for reuse. For instance, I would like to have the lists of components on the summary pages generated as well.

What if the subsidiary component pages have a "group" attribute, and the summary pages have a matching "summary-group" attribute.

For Ignite:

  • summary page:
:summary-group: ignite
  • each ignite component page:
:group: ignite

I think the summary indexes could then be generated with

indexList::[attributes="group={summary-group}"]

I guess the gulpfile would need to be a little more complicated as it would need to read the first few lines of each adoc and at least partially parse the attributes.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

The attribute naming could be used to even link back and forth on the pages as well. Apart from that, I came up with the basic logic to use the file path and title for createComponentNav() function in gulpfile.js. SummaryTitle is an additional variable I assigned.

const filepath = path.basename(filename);
const title = titleFrom(file);
if (filepath.indexOf('summary') > -1) {
      summaryTitle = title.toLowerCase();
} else if (summaryTitle !== '' && filepath.indexOf(summaryTitle) === 0) {
      return `*** xref:${filepath}[${title}]`;
}
return `** xref:${filepath}[${title}]`

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

Within each summary.adoc file that's already present, the sub-components that are listed are written manually aren't they?

@djencks
Copy link
Contributor

djencks commented May 26, 2020

At the moment, yes, and IIRC in lots of different styles. I think it would be nice to generate those lists, which I think is possible with the attributes as I suggested.

Please quote text rather than using screenshots :-)

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

At the moment, yes, and IIRC in lots of different styles. I think it would be nice to generate those lists, which I think is possible with the attributes as I suggested.

Please quote text rather than using screenshots :-)

I am sorry for the screenshot, it was just a method that could be used to generate the nav file for component. I will push the commit, for the change in gulpfile.js I made.

@djencks
Copy link
Contributor

djencks commented May 26, 2020

I appreciate you showing your code, but please use markdown on text; surround the text with ```.

Your code looks like it would work, but I think it is rather fragile. Perhaps we need some more opinions.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 26, 2020

Yes, but for now, I could observe this pattern and come up with this solution. If an another method strikes me, I will discuss it here.

I will push my code as of now.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 27, 2020

I appreciate you showing your code, but please use markdown on text; surround the text with ```.

Your code looks like it would work, but I think it is rather fragile. Perhaps we need some more opinions.

@djencks , as the summary method is fragile, we could use the attribute naming that you suggested. What we could do is as each component is manually written and each has constant variable declared as docTitle, what we could do is divide files into parentGroup and childGroup variables, and check whether it is parent attribute or child using the following logic.

function isParentAttribute(file) {
    var attributeName = /(?::parentGroup: )(.*)/.exec(file.contents.toString())
    if (attributeName != null) {
        return attributeName[1];
    }
    return null;
}

function isChildAttribute(file) {
    var attributeName = /(?::childGroup: )(.*)/.exec(file.contents.toString())
    if (attributeName != null && attributeName[1] == parentAttribute) {
        return true;
    }
    return false;
}

Then while the distribution logic takes place to generate the adoc file, we could use the following logic of if and else:

if (isParentAttribute(file) != null) {
     parentAttribute = isParentAttribute(file);
} else if (isChildAttribute(file)) {
     return `*** xref:${filepath}[${title}]`;
}
return `** xref:${filepath}[${title}]`;

With this manner, it would be accurate and the code will never fail for any test. This method works as I tried it on my code editor. If you like this, we could follow up with this.

@djencks
Copy link
Contributor

djencks commented May 27, 2020

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!

@AemieJ
Copy link
Contributor Author

AemieJ commented May 27, 2020

@djencks For providing the attribute for the ones with relative groupings, the docs pages within the docs/components is auto-generated from the src files in components folder, right?

@djencks
Copy link
Contributor

djencks commented May 27, 2020

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

@AemieJ
Copy link
Contributor Author

AemieJ commented May 28, 2020

@djencks I have included the attributes for the related files within the components/src folders and modified the gulpfile.js, check it out.

@djencks
Copy link
Contributor

djencks commented May 28, 2020

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? I think generating index lists on these pages should be a separate issue.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 28, 2020

  • About the first point, in the gulpfile, the file is generated one by one, and hence I read that turn by turn to check whether they are either the parent or child attribute.

  • Yes, I do want to include aws, aws2 and Google summary pages, it's just modified them within the docs/components inspite of the components folder. I will do that.

  • Also for index lists is being taken into consideration to generate the sub components automatically into the parent component, am I right? It's better if taken into a separate issue for that and keep this one as a separate one.

@djencks
Copy link
Contributor

djencks commented May 28, 2020

  • 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.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 28, 2020

I realized, I made the mistake for two of the group files and omitted the space.

About your third point you mentioned, I made changes to the files within the components src folder, which is the original source code for each documentation, right?

@AemieJ
Copy link
Contributor Author

AemieJ commented May 28, 2020

@djencks I modified the gulpfile to include checkAttributeExistence() and thus it resulted in removal of isChildAttribute() function as it isn't required based on the logic then and I modified the two adoc files where I removed the space. It feels right now, what do you think?

Also, I wanted to ask whether you want me to create the aws-summary.adoc, aws2-summary.adoc and google-summary.adoc as aws has a really high number of sub-components.

@djencks
Copy link
Contributor

djencks commented May 28, 2020

  • 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.

@AemieJ
Copy link
Contributor Author

AemieJ commented May 28, 2020

See, we used summary and group majorly for index listing in future so we could auto generate the list in summary group, so while checking you just required the need for childAttribute and provide a depth of 3. All rest will require a depth of 2.

Also, for adding those additional summary pages, I will do as of now to include the layout similar to other summary pages just for bringing the relativeness in the picture.

@djencks
Copy link
Contributor

djencks commented May 30, 2020

I wrote #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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants