Skip to content

docs: add overrides and jobManifestPath to verify docs#8762

Merged
aaron-prindle merged 1 commit into
GoogleContainerTools:mainfrom
maggieneterval:verify
May 19, 2023
Merged

docs: add overrides and jobManifestPath to verify docs#8762
aaron-prindle merged 1 commit into
GoogleContainerTools:mainfrom
maggieneterval:verify

Conversation

@maggieneterval

Copy link
Copy Markdown
Contributor

Fixes: #8743

Description
First pass at improving skaffold verify documentation:

  • Clarify the difference between local and Kubernetes execution modes.
  • Specify how to customize Kubernetes test Jobs using the new overrides and jobManifestPath options.
@maggieneterval

Copy link
Copy Markdown
Contributor Author

@dorbin PTAL - thanks!

@maggieneterval

Copy link
Copy Markdown
Contributor Author

@dorbin and @aaron-prindle #8743 also requests that jobManifestPath and overrides be included in the YAML schema documentation, but I am already seeing them show up for https://skaffold.dev/docs/references/yaml/?version=v4beta4#verify and https://skaffold.dev/docs/references/yaml/?version=v4beta5. That said, they don't show up for the default (latest stable?) version of the YAML schema page. Wanted to confirm that was expected. Thanks!

@codecov

codecov Bot commented May 9, 2023

Copy link
Copy Markdown

Codecov Report

Merging #8762 (e7fabe5) into main (290280e) will decrease coverage by 6.07%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##             main    #8762      +/-   ##
==========================================
- Coverage   70.48%   64.41%   -6.07%     
==========================================
  Files         515      617     +102     
  Lines       23150    31171    +8021     
==========================================
+ Hits        16317    20079    +3762     
- Misses       5776     9589    +3813     
- Partials     1057     1503     +446     
Impacted Files Coverage Δ
cmd/skaffold/app/cmd/completion.go 13.04% <0.00%> (-1.25%) ⬇️
cmd/skaffold/app/cmd/config/list.go 65.21% <ø> (ø)
cmd/skaffold/app/cmd/config/set.go 88.72% <ø> (ø)
cmd/skaffold/app/cmd/config/util.go 54.28% <ø> (ø)
cmd/skaffold/app/cmd/credits.go 100.00% <ø> (ø)
cmd/skaffold/app/cmd/credits/export.go 0.00% <0.00%> (ø)
cmd/skaffold/app/cmd/deploy.go 40.90% <0.00%> (-12.94%) ⬇️
cmd/skaffold/app/cmd/generate_pipeline.go 60.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_modules.go 65.00% <ø> (ø)
cmd/skaffold/app/cmd/inspect_profiles.go 66.66% <ø> (ø)
... and 40 more

... and 407 files with indirect coverage changes

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

image: integration-test-container
executionMode:
kubernetesCluster: {}
kubernetesCluster:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be commented out w/ some indication the field is optional?

@maggieneterval maggieneterval May 10, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I made a couple changes to help clarify that these fields are optional:

  1. Changed wording on line 28 to "There are two ways to optionally customize the Skaffold-generated Kubernetes Job"
  2. Added "optional" to describe both override configuration options on lines 37 and 38
  3. Commented out the optional lines in the YAML and added "[optional]" above to further clarify.

Let me know what you think!

Comment thread docs-v2/content/en/samples/verify/verify.yaml Outdated

@aaron-prindle aaron-prindle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@dorbin dorbin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, Maggie! Thanks!

@aaron-prindle aaron-prindle merged commit f0eb238 into GoogleContainerTools:main May 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 participants