-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Regenerate for 1.30, fix patches #3680
Regenerate for 1.30, fix patches #3680
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
f1432c3
to
bf396ae
Compare
bf396ae
to
077ed83
Compare
Ok, I updated this so that empty lists are omitted from YAML dumping and I think that should be ok. |
077ed83
to
b15e806
Compare
it looks like k/k api added a new dry-run option for delete API, so we need to align with that. i will append another commit to fix it |
08e8fa2
to
5dac2ce
Compare
@yue9944882 this is fixed up and ready to review, I tried to keep the commits separate for easier reviewing, but I'll likely squash them in the end to fix the issues with authorship and the PR bot. I had to remove a check from |
36ed886
to
3844f99
Compare
This PR failed CLA check due to bot commit, let me squash the branch into single commit to workaround |
Signed-off-by: Min Jin <minkimzz@amazon.com>
3844f99
to
b552ede
Compare
manual lgtm, since I'm the original author of the PR. |
@yue9944882 I regenerated for 1.30 including the fixes for nullable arrays, also fixed up the patches so they applied cleanly.
Ref #3076
Please take a look.Tests are failing b/c the YAML is being generated with empty lists intead of nulls, I'm not sure this is easily fixable but will investigate.YAML is fixed, but one JSON test is failing and needs a fix.This is ready for review.