Skip to content

Properly generate EIP nav.adoc#12146

Merged
zregvart merged 1 commit intoapache:mainfrom
zregvart:pr/eips-nav-regen
Nov 23, 2023
Merged

Properly generate EIP nav.adoc#12146
zregvart merged 1 commit intoapache:mainfrom
zregvart:pr/eips-nav-regen

Conversation

@zregvart
Copy link
Copy Markdown
Member

Description

Seems that the EIP nav.adoc, prior to CAMEL-20135, and it seems not entirely resolved by CAMEL-20135, was not generated properly. This attempts to generate it by including all the Asciidoc from the EIP module within the nav.adoc.

The notion of Asciidoc module without a source, i.e. one that doesn't need to be symlinked or manipulated in any way by Gulp is introduced. This is to provide the list of files to createNav correctly.

Target

  • I checked that the commit is targeting the correct branch (note that Camel 3 uses camel-3.x, whereas Camel 4 uses the main branch)

Tracking

  • If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I have run mvn clean install -DskipTests locally and I have committed all auto-generated changes

@github-actions
Copy link
Copy Markdown
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟

🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run

  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot.

  • You can label PRs using build-all, build-dependents, skip-tests and test-dependents to fine-tune the checks executed by this PR.

  • Build and test logs are available in the Summary page. Only Apache Camel committers have access to the summary.

  • ⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

@github-actions github-actions Bot added the core label Nov 22, 2023
@zregvart
Copy link
Copy Markdown
Member Author

@klease can you have a look at this approach?

@klease
Copy link
Copy Markdown
Contributor

klease commented Nov 22, 2023

  1. I thought the :eips: in the xrefs was required. But maybe not in the nav file itself?
  2. Not being a js expert, I'm not sure, but it looks to me like your change will add a createNav task both with and without a filter for all the sources which have both a json and an asciidoc with source property, and they all do except eips.

Seems that the EIP nav.adoc, prior to CAMEL-20135, and it seems not
entirely resolved by CAMEL-20135, was not generated properly. This
attempts to generate it by including all the Asciidoc from the EIP
module within the nav.adoc.

The notion of Asciidoc module without a source, i.e. one that doesn't
need to be symlinked or manipulated in any way by Gulp is introduced.
This is to provide the list of files to `createNav` correctly.
@zregvart
Copy link
Copy Markdown
Member Author

  • I thought the :eips: in the xrefs was required. But maybe not in the nav file itself?

xref can contain only the name of the file if that file resides in the same module and component version, and in this case nav.adoc and other .adoc are in the same eips module.

  • Not being a js expert, I'm not sure, but it looks to me like your change will add a createNav task both with and without a filter for all the sources which have both a json and an asciidoc with source property, and they all do except eips.

Yeah, I've added an explicit !asciidoc.source, though Gulp maintains unique names of tasks, so the nav:asciidoc:eips task in the json branch should override the task in the asciidoc branch. I think having the explicit condition there helps though, at least when trying to grok the code.

@klease
Copy link
Copy Markdown
Contributor

klease commented Nov 23, 2023

@zregvart Thanks for the explanations and the fix!

@zregvart
Copy link
Copy Markdown
Member Author

@zregvart Thanks for the explanations and the fix!

Thanks for having a look @klease :)

@zregvart zregvart merged commit fad60b7 into apache:main Nov 23, 2023
@zregvart zregvart deleted the pr/eips-nav-regen branch November 23, 2023 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants