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.

Sign up for GitHub

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

Jump to bottom

[V3] (Part 2) Add new schema to latest/v2 #5729

Merged
merged 1 commit into from
Apr 29, 2021
Merged

[V3] (Part 2) Add new schema to latest/v2 #5729

merged 1 commit into from
Apr 29, 2021

Conversation

yuwenma
Copy link
Contributor

@yuwenma yuwenma commented Apr 27, 2021 •
edited
Loading

Related: #5727

Description

  • Define a new schema v3alpha1 for the skaffold v2 feature.
  • Create latest/v2 dir to track v3alpha* schemas.

Follow up things

  • Generate new document page for v3
  • Update pkg/skaffold/schemas/versions.go to sort v1 and v2 independently.

@google-cla google-cla bot added the cla: yes label Apr 27, 2021
@codecov
Copy link

codecov bot commented Apr 27, 2021 •
edited
Loading

Codecov Report

Merging #5729 (a9a336d) into master (03e9a20) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5729      +/-   ##
==========================================
- Coverage   70.68%   70.64%   -0.04%     
==========================================
  Files         423      427       +4     
  Lines       16168    16174       +6     
==========================================
- Hits        11428    11426       -2     
- Misses       3900     3907       +7     
- Partials      840      841       +1
Impacted Files Coverage Δ
pkg/skaffold/schema/latest/v2/config.go 0.00% <0.00%> (ø)
pkg/skaffold/schema/latest/v2/upgrade.go 0.00% <0.00%> (ø)
pkg/skaffold/schema/v3alpha1/config.go 0.00% <0.00%> (ø)
pkg/skaffold/schema/v3alpha1/upgrade.go 0.00% <0.00%> (ø)
pkg/skaffold/util/tar.go 50.66% <0.00%> (-5.34%) ⬇️
pkg/skaffold/docker/parse.go 87.14% <0.00%> (+0.95%) ⬆️

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 03e9a20...a9a336d. Read the comment docs.

@yuwenma yuwenma added the area/v3 V3 label Apr 27, 2021
@yuwenma yuwenma force-pushed the schema-refactor-2 branch 3 times, most recently from ba4b1c0 to 1ed5265 Compare April 28, 2021 18:18
tejal29
tejal29 reviewed Apr 28, 2021
View reviewed changes
pkg/skaffold/schema/latest/v2/config.go Outdated
Test []*latest_v1.TestCase `yaml:"test,omitempty"`

// Render describes how the original manifests are hydrated, validated and transformed.
Render RenderConfig `yaml:"render,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, just saw the config in detail now.
Shd we instead use ?

manifests:
   
    validate:

    transform:

Copy link
Contributor Author

@yuwenma yuwenma Apr 28, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I used the field name from the design doc. Do we get a conclusion about the render UX? I can renaming these fields once we determine the final names ( design doc needs update). For now, this is just to align with the design doc. Whom do you suggest determining (documenting) the render field names from the survey?

@tejal29 @viglesiasce

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the UX from "render" to "manifests" and keep the struct name as "render" (manifests seem to be over-used). I can rename the fields if there're some better choices coming up.

@tejal29 tejal29 added the docs-modifications runs the docs preview service on the given PR label Apr 28, 2021
@container-tools-bot
Copy link

Please visit http://35.236.9.12:1313 to view changes to the docs.

@container-tools-bot container-tools-bot removed the docs-modifications runs the docs preview service on the given PR label Apr 28, 2021
@tejal29 tejal29 enabled auto-merge (squash) April 29, 2021 21:55
@yuwenma
[V3] (Part 2) Add new schema to latest/v2
a9a336d 
- Define a new schema `v3alpha1` for the skaffold v2 feature.
- Create `latest/v2` dir to track `v3alpha*` schemas.
@tejal29 tejal29 merged commit d5acad2 into GoogleContainerTools:master Apr 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tejal29 tejal29 tejal29 approved these changes

Assignees
No one assigned
Labels
area/v3 V3 cla: yes size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@yuwenma @container-tools-bot @tejal29