[test_operator] Automatic sync of README.md#3589
[test_operator] Automatic sync of README.md#3589sauragar wants to merge 5 commits intoopenstack-k8s-operators:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
kstrenkova
left a comment
There was a problem hiding this comment.
Thank you for the PR! I have tested it locally and found a small issue with updating the defaults. It is explained in the comments, the fix should be rather simple. Other than that I saw the README sync was working nicely, so I think you're close to the finish 🎉 I like that the parameters will be in alphabetical order now :D Maybe there could be a mention of the script somewhere in the README so that people know they can use the sync when creating new variables or updating the old ones.
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/4de28a813ea54e73883c28ab7ff35c81 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 18m 29s |
kstrenkova
left a comment
There was a problem hiding this comment.
I am adding 2 comments about the same issue atop of my previous comments.
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/fe0fdd8386324989bd8ea335a95f52d6 ❌ openstack-k8s-operators-content-provider FAILURE in 4m 53s |
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/5eac463796534f4b89b1f990b236fe22 ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 29m 11s |
f256f7e to
c6e4dd2
Compare
082fce7 to
f2cf98b
Compare
- Add automatic sync script (sync_test_operator_var_readme.py) to keep README.md in sync with defaults/main.yml - Reorganize parameter documentation in alphabetical order - Replace inline default value definitions with references to defaults/main.yml to reduce duplication and improve maintainability - Add auto-sync note at the top of README.md - Fix pre-commit ansible-lint compatibility Signed-off-by: Saurabh Agarwal <sauragar@redhat.com> Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
kstrenkova
left a comment
There was a problem hiding this comment.
I am leaving a few very small comments. I think the PR is almost done, so it would be a good idea to ask someone else for a review to get more perspective. I am also not sure about editing the pre-commit and setup_molecule files, we need someone from ci-framework for that :D
|
|
||
| def normalize_default_text(block, value): | ||
| # Normalize "Defaults to `x`" → "Default value: `x`" | ||
| block = re.sub( |
There was a problem hiding this comment.
I don't think this substitution is needed. If someone wanted to, they could also do Default: and this code wouldn't help to keep consistency. So I don't think it's necessary to normalize it, as you can never catch all cases.
michburk
left a comment
There was a problem hiding this comment.
A handful of comments and suggestions.
Overall, I understand the motivation to keep the readme up to date by using a script. We've done a pretty poor job of enforcing that the docs be updated with every change to the defaults, but I don't love the idea of designing and maintaining a script specifically for just one role's readme. I'd be curious to see what other cifmw folks have to say.
| * `cifmw_test_operator_logs_image`: (String) Image that should be used to collect logs from the pods spawned by the test-operator. Default value: `quay.io/quay/busybox` | ||
| ## Generic parameters | ||
| <!-- START GENERIC_PARAMETERS --> | ||
| * `cifmw_test_operator_artifacts_basedir`: (String) Directory where we will have all test-operator related files. Default value: `{{ cifmw_basedir }}/tests/test_operator` which Default value: `~/ci-framework-data/tests/test_operator` |
There was a problem hiding this comment.
Wording here is weird: "Default value: ... which Default value: ..."
…riables Update the README sync script to show long single-line default values inline instead of redirecting to defaults/main.yml. Only complex values (lists, dicts, multiline strings) now redirect to the defaults file. This improves documentation readability, especially when viewed on web browsers where links to defaults/main.yml may not work properly. Changes: - Remove length check from should_redirect() function - Display template strings inline regardless of length - Remove unused MAX_INLINE_LENGTH and MAX_INLINE_LINES constants - Clean up debug print statements and comments - Update README with inline default values Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/e6bd5c4155b14750817cdc542eae5cdb ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 25m 36s |
Update the README sync script to generate absolute GitHub URLs instead of relative paths for "See defaults in main.yml" references. This fixes broken links when the documentation is rendered on readthedocs. Previously, links like [main.yml](defaults/main.yml) would not work on the readthedocs website because the YAML file is not included in the documentation build. Now all links point to: https://github.com/openstack-k8s-operators/ci-framework/blob/main/roles/test_operator/defaults/main.yml Changes: - Update DEFAULTS_LINK to use absolute GitHub URL - Add logic to update existing relative links to GitHub URLs - Update DEFAULTS_REGEX to match both old and new link formats - Update all 17 "See defaults in" links in README Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Update the README sync script to include line number anchors in GitHub URLs for "See defaults in" links. This allows users to jump directly to the specific variable definition instead of just opening the file. The script now: - Tracks line numbers for each variable when loading defaults/main.yml - Generates GitHub URLs with line anchors (e.g., #L35 or #L46-L61) - Links single-line variables to a specific line (e.g., #L35) - Links multi-line variables to a line range (e.g., #L46-L61) Example changes in README: - cifmw_test_operator_default_groups → main.yml#L35 - cifmw_test_operator_log_pod_definition → main.yml#L46-L61 - cifmw_tempest_tempestconf_config_defaults → main.yml#L98-L132 This significantly improves user experience when viewing documentation on readthedocs, as clicking links now highlights the exact variable. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/f84c25fe639c4a2eaf5b977d149e5a2f ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 21m 51s |
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
Build failed (check pipeline). Post https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/5e37469eb30e4f58babd13c526c9336d ✔️ openstack-k8s-operators-content-provider SUCCESS in 2h 43m 11s |
This commit will ensure to have the proper auto-sync of README.md with the defaults/main.yml file and also added the post-commit file so whenever someone commits in the defaults/main.yml file the script can be auto-run
Ticket: OSPRH-19423
This PR #3327 is closed since I am not able to reopen the PR
Signed-off-by: Saurabh Agarwal sauragar@redhat.com