Skip to content

Conversation

MahnoorAsghar
Copy link
Contributor

@MahnoorAsghar MahnoorAsghar commented Sep 25, 2025

Add prow job in BMO to generate CR documentation; and
Revert #1107 'Install podman in basic-checks container'
This should work alongside metal3-io/baremetal-operator#2605

@metal3-io-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign kashifest for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 25, 2025
@MahnoorAsghar MahnoorAsghar force-pushed the prow-cr-doc-bmo branch 2 times, most recently from f159c0b to 533546e Compare September 25, 2025 10:10
Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

This will generate the doc, but its missing the part where it detects if something has changed. That is easier to implement if you look at the existing hack/ scripts how they deal with local run and the prow run.

@MahnoorAsghar
Copy link
Contributor Author

This will generate the doc, but its missing the part where it detects if something has changed. That is easier to implement if you look at the existing hack/ scripts how they deal with local run and the prow run.

@tuminoid I see that the pattern in hack/ is to launch a podman container, and once in it we execute a command, but I didn't understand what you meant by the detecting if something has changed part? Does the 'run_if_changed' prow argument not take care of that?
(I can ofcourse add a script to the hack/ directory for consistency or benefits)

@tuminoid
Copy link
Member

This will generate the doc, but its missing the part where it detects if something has changed. That is easier to implement if you look at the existing hack/ scripts how they deal with local run and the prow run.

@tuminoid I see that the pattern in hack/ is to launch a podman container, and once in it we execute a command, but I didn't understand what you meant by the detecting if something has changed part? Does the 'run_if_changed' prow argument not take care of that? (I can ofcourse add a script to the hack/ directory for consistency or benefits)

The point of crdoc generation in tests is to see if rerunning it produces a diff, which would mean the author did not include the updated documentation in their PR. So it needs to fulfill two things:

  1. allow developer to generate the updated API docs and add it to their PR
  2. re-generate the API docs in prow, and FAIL if there was any change

- name: generate-api-docs
branches:
- main
run_if_changed: '(config/base/crds/bases/.*|^Makefile)$*'
Copy link
Member

Choose a reason for hiding this comment

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

Also needs to run if the gen-api-docs.sh script itself changes, so we validate it doesn't break. Sorry forgot that one first time around.

Copy link
Member

@tuminoid tuminoid left a comment

Choose a reason for hiding this comment

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

One more comment, and also please fix the commit and PR title and description.

@MahnoorAsghar MahnoorAsghar changed the title Add prow job to generate CR documentation in BMO; and revert 'Install… Add prow job in BMO to generate CR documentation Sep 26, 2025
@tuminoid
Copy link
Member

/override metal3-ubuntu-e2e-integration-test-main

@metal3-io-bot
Copy link
Collaborator

@tuminoid: Overrode contexts on behalf of tuminoid: metal3-ubuntu-e2e-integration-test-main

In response to this:

/override metal3-ubuntu-e2e-integration-test-main

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@tuminoid
Copy link
Member

/hold
until metal3-io/baremetal-operator#2605 is completed

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants