Skip to content
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

Automatically infer latest spec revision when importing deployments #1079

Merged
merged 3 commits into from
Mar 10, 2023

Conversation

timburks
Copy link
Contributor

@timburks timburks commented Mar 10, 2023

Fixes #1078

tests are coming in this PR

@@ -39,11 +39,27 @@ func relativeSpecRevisionName(apiName names.Api, spec string) string {
}

// optionalSpecRevisionName returns a spec revision name if the subpath is not empty
func optionalSpecRevisionName(deploymentName names.Deployment, subpath string) string {
func optionalSpecRevisionName(ctx context.Context, client *gapic.RegistryClient, deploymentName names.Deployment, subpath string) (string, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is quite a vague function name, we should make sure the comment fully documents what to expect.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, it should probably be renamed. I'll suggest something in the next commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolveSpecRevisionName with a better comment.

@codecov
Copy link

codecov bot commented Mar 10, 2023

Codecov Report

Merging #1079 (e4d8791) into main (b9f77d1) will decrease coverage by 0.02%.
The diff coverage is 57.89%.

@@            Coverage Diff             @@
##             main    #1079      +/-   ##
==========================================
- Coverage   68.01%   67.99%   -0.02%     
==========================================
  Files         147      147              
  Lines       11975    11990      +15     
==========================================
+ Hits         8145     8153       +8     
- Misses       3131     3136       +5     
- Partials      699      701       +2     
Impacted Files Coverage Δ
cmd/registry/patch/deployment.go 65.76% <57.89%> (-1.95%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Registry tool: fix registry apply to automatically infer spec revision references in deployment YAML
2 participants