-
Notifications
You must be signed in to change notification settings - Fork 50
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
Update resource-manager.hcl for nginx new version #4382
Conversation
removed trailing n
Thanks for doing this @zaowang-ms . This looks good to me. Here's the link to the swagger spec 2024-06-01-preview: |
@catriona-m Thank you for starting to review this PR. This changeset looks very similar to https://github.com/hashicorp/pandora/pull/3830/files from @tombuildsstuff . I can't think of why the unit test would fail to import the swagger spec. Can you please share some pointers on how to start debugging this? |
@zaowang-ms I think I see the problem. Can you please update your feature branch on your own fork to latest main of this repository? It appears to be the case that your fork has out of date sub modules and you should sync it with the upstream (this repository) to get the unit tests to pass. |
@catriona-m This is an interesting issue around the fork being out of sync from the upstream (this repository). Do you think the hashicorp team can do something here to do a fail fast check during PRs to make sure that the author knows quickly what might be wrong with spec imports? If this already exists, can you please share that with me here? Thanks!! |
@catriona-m I see @zaowang-ms updated the fork and it now has the latest swagger submoduled. Can you please trigger the workflows again. I think they will pass now. |
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.
Thanks @zaowang-ms & @puneetsarna - the tests are passing now and this LGTM 👍
This is the PR updating available versions of nginx on Azure.