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

Add more comments to the Config structs #1630

Merged
merged 1 commit into from
Feb 11, 2019

Conversation

dgageot
Copy link
Contributor

@dgageot dgageot commented Feb 8, 2019

Signed-off-by: David Gageot [email protected]

@codecov-io
Copy link

Codecov Report

Merging #1630 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1630   +/-   ##
======================================
  Coverage    46.9%   46.9%           
======================================
  Files         119     119           
  Lines        5134    5134           
======================================
  Hits         2408    2408           
  Misses       2478    2478           
  Partials      248     248

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 5862895...703ed78. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Feb 8, 2019

Codecov Report

Merging #1630 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1630   +/-   ##
======================================
  Coverage    46.9%   46.9%           
======================================
  Files         119     119           
  Lines        5134    5134           
======================================
  Hits         2408    2408           
  Misses       2478    2478           
  Partials      248     248

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 19edc4e...f02d70c. Read the comment docs.

@@ -44,17 +54,44 @@ func (c *SkaffoldPipeline) GetVersion() string {

// BuildConfig contains all the configuration for the build steps
type BuildConfig struct {
// Artifacts lists the images you're going to be building
// you can include as many as you want here.
Copy link
Contributor

Choose a reason for hiding this comment

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

missing a comma or semicolon or new sentence here

GoogleCloudBuild *GoogleCloudBuild `yaml:"googleCloudBuild,omitempty" yamltags:"oneOf=build"`
KanikoBuild *KanikoBuild `yaml:"kaniko,omitempty" yamltags:"oneOf=build"`

// KanikoBuild describes how to do a on-cluster build using
Copy link
Contributor

Choose a reason for hiding this comment

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

"an on-cluster..."

// GoogleCloudBuild describes how to do a remote build on
// [Google Cloud Build](https://cloud.google.com/cloud-build/docs/).
// Docker and Jib artifacts can be built on Cloud Build. The `projectId` needs
// to be provided and the currently logged user should be given permissions to trigger
Copy link
Contributor

Choose a reason for hiding this comment

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

logged in ?

// Defaults to `localDir`.
BuildContext *KanikoBuildContext `yaml:"buildContext,omitempty"`

// Cache
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm

Cache *KanikoCache `yaml:"cache,omitempty"`

// AdditionalFlags
AdditionalFlags []string `yaml:"flags,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm

Version string `yaml:"version,omitempty"`

// SetValues a list of key-value pairs.
// If present, Skaffold will sent `--set` flag to Helm CLI and append all pairs after the flag.
Copy link
Contributor

Choose a reason for hiding this comment

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

will send

// Defaults to `false`.
RecreatePods bool `yaml:"recreatePods,omitempty"`

// SkipBuildDependencies
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm

}

// JibMavenArtifact builds containers using the Jib plugin for Maven.
Copy link
Contributor

Choose a reason for hiding this comment

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

These are different style than BazelArtifact and DockerArtifact

@dgageot
Copy link
Contributor Author

dgageot commented Feb 9, 2019

Thanks for the help @jonjohnsonjr!

@balopat balopat merged commit b2b99a7 into GoogleContainerTools:master Feb 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants