-
Notifications
You must be signed in to change notification settings - Fork 143
[test_operator] Automatic sync of README.md #3589
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
base: main
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| * `cifmw_test_operator_controller_ip`: (String) An ip address of the controller node. Default value: `ansible_default_ipv4.address` which defaults to (`""`). | ||
| * `cifmw_test_operator_tolerations`: (Dict) `tolerations` value that is applied to all pods spawned by the test-operator and to the test-operator-controller-manager and test-operator-logs pods. Default value: `{}` | ||
| * `cifmw_test_operator_default_groups`: (List) List of groups in the include list to search for tests to be executed. Default value: See defaults in [main.yml](defaults/main.yml) | ||
| * `cifmw_test_operator_default_image_tag`: (String) Default tag for all the supported test frameworks (tempest, tobiko, horizontest and ansibletest) to pull their container images. It can be overridden at test framework level. Defaults to `current-podified` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your script won't work on parameters that do not use Default value: specifically. So for example this parameter will not get synced because it has Defaults to .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the script will not be able to handle this because there are some variables which have both Default value: and defaults to in the same variable and in this case we can remove defaults to wherever its present and replace it with Defaults to: then the script can work properly
For example: * cifmw_test_operator_controller_ip: (String) An ip address of the controller node. Default value: ansible_default_ipv4.address which defaults to ("").
If there are any suggestions then we can discuss.
| return existing_block | ||
|
|
||
| # CASE 2: New variable | ||
| if should_redirect(value): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you create a new variable there is no text. Your script woks only if the explanation has Default value: so I would recommend adding it when creating a new variable.
For example, cifmw_test: 10 has been added to defaults, so * cifmw_test: Default value: 10 has been added to README.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this issue in the updated code
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 openstack-k8s-operators#3327 is closed since I am not able to reopen the PR Signed-off-by: Saurabh Agarwal <sauragar@redhat.com>
|
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 |
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
Signing off: Saurabh Agarwal sauragar@redhat.com