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.

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

[CI] Remove Maven profile full in favour of using docs for all document generation. #6007

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

corneil
Copy link
Contributor

@corneil corneil commented Oct 23, 2024

No description provided.

@corneil corneil requested a review from cppwfs October 23, 2024 10:09
@@ -17,7 +17,7 @@ jobs:
shell: bash
timeout-minutes: 75
run: |
./mvnw -B -s .github/settings.xml -Pdocs clean install --no-transfer-progress
./mvnw -B -s .github/settings.xml -Pfull,docs clean install --no-transfer-progress
Copy link
Contributor

Choose a reason for hiding this comment

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

What is full vs. docs? I guess I don't understand how full fulfills the PR description of executes rest-docs generation as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't choose the name. We can change to rest-docs to ensure it is unambiguous.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having checked:

  • docs means javadocs
  • full means all rest-docs and asciidocs.

How about we rather use javadocs and fulldocs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me . @onobc what do you think?

Copy link
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

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

@corneil, Love the renames.
But, when we make these changes we will have to update the following files:

  • ci.yml
  • build-snapshot-worker.yml
  • build-docs.yml
  • spring-cloud-skipper/README.adoc
  • appendix-building.adoc

@corneil corneil changed the title [CI] Ensure CI-PR executes rest-docs generation as well. [CI] Remove Maven profile full in favour of using docs for all document generation. Oct 28, 2024
@corneil corneil requested review from cppwfs and onobc October 28, 2024 12:31
<skipTests>false</skipTests>
<includes>
<include>**/*Documentation.java</include>
<include>**/*Tests.java</include>
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably want to include the **/*Tests.java to the docs plugin below. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed to the surefire in docs profile to use the package phase which means test phase executes normal unit tests and package phase generates the snippets.
When -DskipTests is present the normal test phase is skipped. When -Pdocs is not provides then the snippets are not generated.

Copy link
Contributor

@cppwfs cppwfs left a comment

Choose a reason for hiding this comment

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

Looks Great! One small question below.

… skip tests and generate documentation but allow for normal tests to execute id -DskipTests are not specified.
…incorrect assumption of where the resources are located.

Disable test suites that are not needed to create documentation snippets.
@corneil corneil requested a review from cppwfs November 1, 2024 15:23
@corneil
Copy link
Contributor Author

corneil commented Nov 1, 2024

The latests commit fixes tests for the actual resources present that do work when doing local deployments.

@corneil
Copy link
Contributor Author

corneil commented Nov 2, 2024

We have a pass locally fails in CI-PR situation. 😢

Improved surefire report plugin config.
Fix last failing tests.
@corneil corneil force-pushed the corneil/ensure-ci-pr-tests-documentation-generation branch from 7cf5109 to d1d71a1 Compare November 5, 2024 09:18
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.

3 participants