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

[v3] Schema Version upgrading for v1 and v2. #5796

Merged
merged 1 commit into from
May 7, 2021

Conversation

yuwenma
Copy link
Contributor

@yuwenma yuwenma commented May 6, 2021

Related: #5673

Description

  1. Add upgrade func to v3alpha1
  2. Enable schema parsing for v2 schemas (v3alpha*)
  3. Compatibility check among v1 and v2 schemas
  4. Update skaffold fix to avoid upgrading v1 schemas to v2 and vice versa.
  5. Update the schemas usage in hack, cmd/fix cmd/list to hide v2 schemas from users.

@yuwenma yuwenma requested a review from a team as a code owner May 6, 2021 13:58
@yuwenma yuwenma requested a review from aaron-prindle May 6, 2021 13:58
@google-cla google-cla bot added the cla: yes label May 6, 2021
@yuwenma yuwenma requested review from MarlonGamez and tejal29 and removed request for aaron-prindle May 6, 2021 13:58
@codecov
Copy link

codecov bot commented May 6, 2021

Codecov Report

Merging #5796 (d463b23) into master (36395cc) will increase coverage by 0.02%.
The diff coverage is 68.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5796      +/-   ##
==========================================
+ Coverage   70.92%   70.95%   +0.02%     
==========================================
  Files         438      438              
  Lines       16434    16477      +43     
==========================================
+ Hits        11656    11691      +35     
- Misses       3921     3926       +5     
- Partials      857      860       +3     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/schema/list.go 93.75% <ø> (ø)
pkg/skaffold/runner/v1/generate_pipeline.go 0.00% <0.00%> (ø)
pkg/skaffold/schema/latest/v2/config.go 100.00% <ø> (+100.00%) ⬆️
pkg/skaffold/schema/v3alpha1/upgrade.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/fix.go 74.41% <20.00%> (-7.64%) ⬇️
pkg/skaffold/schema/versions.go 83.20% <80.70%> (+0.78%) ⬆️
pkg/skaffold/parser/config.go 79.56% <100.00%> (ø)
pkg/skaffold/docker/parse.go 86.19% <0.00%> (-0.96%) ⬇️
pkg/skaffold/util/tar.go 56.00% <0.00%> (+5.33%) ⬆️
pkg/skaffold/schema/v3alpha1/config.go 50.00% <0.00%> (+50.00%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 36395cc...d463b23. Read the comment docs.

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

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

Looking pretty good, have a few nits/comments

cmd/skaffold/app/cmd/schema/list_test.go Outdated Show resolved Hide resolved
pkg/skaffold/schema/versions.go Outdated Show resolved Hide resolved
pkg/skaffold/schema/versions_test.go Outdated Show resolved Hide resolved
pkg/skaffold/schema/versions_test.go Outdated Show resolved Hide resolved
pkg/skaffold/schema/versions.go Outdated Show resolved Hide resolved
@yuwenma yuwenma force-pushed the schema-versions branch from fca3896 to b7d8feb Compare May 6, 2021 20:29
@yuwenma yuwenma requested a review from MarlonGamez May 6, 2021 22:36
1. Add upgrade func to v3alpha1
2. Enable schema parsing for v2 schemas (v3alpha*)
3. Compatibility check among v1 and v2 schemas
4. Update `skaffold fix` to avoid upgrading v1 schemas to v2 and vice versa.
5. Update the schemas usage in `hack`, `cmd/fix` `cmd/list` to hide v2 schemas from users.
@yuwenma yuwenma force-pushed the schema-versions branch from b7d8feb to d463b23 Compare May 7, 2021 17:35
@tejal29 tejal29 enabled auto-merge (squash) May 7, 2021 18:12
@tejal29 tejal29 dismissed MarlonGamez’s stale review May 7, 2021 19:16

addressed concerns

@tejal29 tejal29 merged commit 8efc9ef into GoogleContainerTools:master May 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants