update DEVELOPER_GUIDE.md to ensure the requirement of adding resources in wrapper librabry#513
Merged
dbasunag merged 2 commits intoopendatahub-io:mainfrom Aug 12, 2025
Merged
Conversation
…es in wrapper librabry
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds documentation to DEVELOPER_GUIDE.md detailing how to generate missing resources via class_generator, submit PRs to the wrapper, and require tests/utilities/fixtures to use wrapper resources or oc when appropriate. No code or public API changes. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/wip', '/hold', '/verified', '/cherry-pick', '/lgtm', '/build-push-pr-image'} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Comment on lines
+69
to
+74
| For any missing resources, please generate a new resource using | ||
| [class_generator tool](https://github.com/RedHatQE/openshift-python-wrapper/blob/main/class_generator/README.md) and | ||
| create a PR against wrapper. Calls to cluster resources from tests, utils and fixtures must always use | ||
| openshift-python-wrapper resource or oc command | ||
| (when wrapper resource is not relevant. e.g. must-gather generation) | ||
|
|
Contributor
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Tighten language, fix grammar/consistency, and format oc
Refines phrasing, aligns “utils” to “utilities” (consistent with earlier sections), clarifies “resource class,” adds missing articles/punctuation, and formats oc as a command.
Apply this diff:
-For any missing resources, please generate a new resource using
-[class_generator tool](https://github.com/RedHatQE/openshift-python-wrapper/blob/main/class_generator/README.md) and
-create a PR against wrapper. Calls to cluster resources from tests, utils and fixtures must always use
-openshift-python-wrapper resource or oc command
-(when wrapper resource is not relevant. e.g. must-gather generation)
+For any missing resources, generate a new resource class using the
+[class_generator tool](https://github.com/RedHatQE/openshift-python-wrapper/blob/main/class_generator/README.md),
+and open a PR against the
+[openshift-python-wrapper repository](https://github.com/RedHatQE/openshift-python-wrapper).
+Calls to cluster resources from tests, utilities, and fixtures must use an
+openshift-python-wrapper resource; when a wrapper resource is not relevant
+(e.g., must-gather generation), use the `oc` command.Optionally verify external links resolve (HTTP 200):
🏁 Script executed:
#!/bin/bash
set -euo pipefail
urls=(
"https://github.com/RedHatQE/openshift-python-wrapper/blob/main/README.md"
"https://github.com/RedHatQE/openshift-python-wrapper/blob/main/class_generator/README.md"
)
for u in "${urls[@]}"; do
code=$(curl -s -o /dev/null -w "%{http_code}" "$u")
echo "$code $u"
done
# Expect: 200 for each URLLength of output: 872
docs/DEVELOPER_GUIDE.md (lines 69–74): refine phrasing, fix grammar & consistency
To improve clarity and align with existing terminology, apply this diff:
-For any missing resources, please generate a new resource using
-[class_generator tool](https://github.com/RedHatQE/openshift-python-wrapper/blob/main/class_generator/README.md) and
-create a PR against wrapper. Calls to cluster resources from tests, utils and fixtures must always use
-openshift-python-wrapper resource or oc command
-(when wrapper resource is not relevant. e.g. must-gather generation)
+For any missing resources, generate a new resource class using the
+[class_generator tool](https://github.com/RedHatQE/openshift-python-wrapper/blob/main/class_generator/README.md),
+and open a PR against the
+[openshift-python-wrapper repository](https://github.com/RedHatQE/openshift-python-wrapper).
+Calls to cluster resources from tests, utilities, and fixtures must use an
+openshift-python-wrapper resource; when a wrapper resource is not relevant
+(e.g., must-gather generation), use the `oc` command.- “utilities” replaces “utils” for consistency.
- Added “class” after “resource” for precision.
- Linked both the class_generator tool and the wrapper repo directly.
- Formatted
ocas inline code. - Updated punctuation in parenthetical for standard style.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| For any missing resources, please generate a new resource using | |
| [class_generator tool](https://github.com/RedHatQE/openshift-python-wrapper/blob/main/class_generator/README.md) and | |
| create a PR against wrapper. Calls to cluster resources from tests, utils and fixtures must always use | |
| openshift-python-wrapper resource or oc command | |
| (when wrapper resource is not relevant. e.g. must-gather generation) | |
| For any missing resources, generate a new resource class using the | |
| [class_generator tool](https://github.com/RedHatQE/openshift-python-wrapper/blob/main/class_generator/README.md), | |
| and open a PR against the | |
| [openshift-python-wrapper repository](https://github.com/RedHatQE/openshift-python-wrapper). | |
| Calls to cluster resources from tests, utilities, and fixtures must use an | |
| openshift-python-wrapper resource; when a wrapper resource is not relevant | |
| (e.g., must-gather generation), use the `oc` command. |
🤖 Prompt for AI Agents
In docs/DEVELOPER_GUIDE.md around lines 69–74, refine the paragraph to replace
"utils" with "utilities", add the word "class" after each occurrence of
"resource" for precision, change the existing tool link to directly reference
the class_generator README and also link the wrapper repo homepage, format the
oc command as inline code (i.e., use the project's inline code style for `oc`),
and update the parenthetical punctuation to standard style (e.g., include
periods and commas inside parentheses as needed); make these phrasing and
grammar adjustments while preserving the original meaning about generating
missing resources and using wrapper classes or the oc command when appropriate.
dbasunag
changed the title
adolfo-ab
approved these changes
Aug 12, 2025
jiridanek
requested changes
Aug 12, 2025
Member
There was a problem hiding this comment.
I'm not a fan, I believe that if it is an important requirement, it should be managed the way @russellb did it in vllm in
Hopefully a helpful chatbot, especially when given the vllm code as an example, can generate the pre-commit check with not too much effort.
I believe that documenting this will only help in disciplining the offenders (RTFM you fool!) but it will not have preventative effect for newcomers.
Combination of automated check that references the contributing documentation when failing, is imo the way.
The code to forbid is something like
api_version = "imageregistry.operator.openshift.io/v1"
kind = "Config"
resource = admin_client.resources.get(api_version=api_version, kind=kind)taken from 6a9dc1e#diff-67c9d3198e7209553e9352e8d116d28a2c1008331bf7e391b3acfc26707a98eeR18-R50
Collaborator
Author
|
I like this idea and will see if this is doable at my end. But I would still want the doc to clearly state what is needed. I feel, many of the newer contributors did not know they are required to add such resources and tools exists to create them automatically. |
rhods-ci-bot
added
commented-by-dbasunag
and removed
lgtm-by-adolfo-ab
commented-by-dbasunag
labels
|
Status of building tag latest: success. |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
How Has This Been Tested?
Merge criteria: