-
Notifications
You must be signed in to change notification settings - Fork 584
Fix payload CRD/FetaureGate ordering #2667
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: master
Are you sure you want to change the base?
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
Hello @JoelSpeed! Some important instructions when contributing to openshift/api: |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||
📝 WalkthroughWalkthroughThis change reorganizes Makefile target dependencies for code generation. The 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||
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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Makefile`:
- Line 127: The Makefile line defining the update-non-codegen target contains a
trailing whitespace character at the end of the line; remove the trailing space
after "update-prerelease-lifecycle-gen" on the update-non-codegen line so the
target line reads without any trailing whitespace (look for the target name
"update-non-codegen" in the Makefile).
| # E.g. the payload feature gates which is not part of the generator style, but is still a subcommand. | ||
| .PHONY: update-non-codegen | ||
| update-non-codegen: update-protobuf tests-vendor update-prerelease-lifecycle-gen update-payload-crds update-payload-featuregates | ||
| update-non-codegen: update-protobuf tests-vendor update-prerelease-lifecycle-gen |
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.
Minor: trailing whitespace.
There's a trailing space at the end of line 127.
Proposed fix
-.PHONY: update-non-codegen
-update-non-codegen: update-protobuf tests-vendor update-prerelease-lifecycle-gen
+.PHONY: update-non-codegen
+update-non-codegen: update-protobuf tests-vendor update-prerelease-lifecycle-gen🤖 Prompt for AI Agents
In `@Makefile` at line 127, The Makefile line defining the update-non-codegen
target contains a trailing whitespace character at the end of the line; remove
the trailing space after "update-prerelease-lifecycle-gen" on the
update-non-codegen line so the target line reads without any trailing whitespace
(look for the target name "update-non-codegen" in the Makefile).
everettraven
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.
/lgtm
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: everettraven The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/verified by @JoelSpeed I verified this locally |
|
@JoelSpeed: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
@JoelSpeed: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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. I understand the commands that are listed here. |
User description
A recent change to refactor the makefiles moved things into codegen, and non-codegen. Unfortunately this also accidentally re-ordered some things so that their dependencies were not in the right order.
This updates the scripts to move the payload updates, since those depend on codegen to have run, into the codegen target
PR Type
Bug fix
Description
Move payload CRD and FeatureGate generation into codegen target
Ensure payload updates run after codegen generators complete
Fix dependency ordering in Makefile targets
Diagram Walkthrough
File Walkthrough
Makefile
Reorder payload generation into codegen targetMakefile
update-payload-crdsandupdate-payload-featuregatesfromupdate-non-codegentarget toupdate-codegentargetafter codegen generators
codegen completion