-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Final* go rewrite services #11593
Final* go rewrite services #11593
Conversation
…added during recent commits
… allow empty object
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 2 Click here to see the affected service packages
Tests were added that are skipped in VCR:
View the build log |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 2 Click here to see the affected service packages
Tests were added that are skipped in VCR:
View the build log |
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 20 Click here to see the affected service packages
Action takenFound 12 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
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 assuming tests pass
While waiting on the tests I wanted to call attention to a specific change: https://github.com/GoogleCloudPlatform/magic-modules/pull/11593/files#diff-d4257ecf68b40cde4cd0e6f0b557a06b872641c57dbda2bcc459c1a6691013ff This satisfied the new issue I ran into from your original change (bd00206#diff-d4257ecf68b40cde4cd0e6f0b557a06b872641c57dbda2bcc459c1a6691013ff) that was a deviation from the original Ruby logic, but because your version caused a change in a resource I went over, it potentially also introduced diffs in other resources. Hopefully the way I altered it pre-emptively fixed those too, but something to keep in mind if we see new diffs after syncing it all |
|
test failure looks to be a problem of all the netapp tests being ran together |
This should be good to go |
All remaining services
a good number of hand written files needed minor syncing for services that were updated since the handwritten templates were batch converted. but since a lot of handwritten files have been minorly updated for whitespace, odds are we will need to re-sync them by hand for ones that need to be re-updated
I think the large yaml regeneration is the next step here, but hopefully would not result in any "new" diffs from the minor ones we accepted as inevitable
Release Note Template for Downstream PRs (will be copied)