feat(helm): add create command to 'helm dependency'

[larryrensing]

When 'helm dependency create ' is run, it will create a new dependency in the requirements.yaml of the current chart path. If there is no requirements.yaml found, it will create the file and add the provided dependency to the file. Users can specify a version -v (default: 0.1.0) and a chart path -c (default: current directory)

Closes: #1947

[prydonius]

This is really great, gave it a quick try. Couple of things I noticed:

• Perhaps the default version should be "> 0" as I got a "could not get a valid version for redis" when I tried it out with specifying the version.
• Currently creating a dependency with the same name creates a new entry, it would be useful (and perhaps more correct) if it updated the existing one
• Maybe the command should be called add instead of create, similar to helm repo add?

[thomastaylor312]

This looks pretty good. I agree with all of @prydonius' points above. I definitely think the behavior should be to update the version of a chart instead of adding an extra one before we merge. Let me know what you think about the other feedback as well.

[thomastaylor312]

It might be better to have this get the latest version (like what helm get does)

[wilkers-steve]

Yeah, after some thought, I agree with you @thomastaylor312. Defaulting 'version' to a value that may not exist in the repository could result in unexpected behavior. I think latest does make sense here.

[prydonius]

I think latest can be defined here by > 0

[technosophos]

I am going to suggest not using -v for the short flag. Here are all the other cases where we define a --version:

⇒  ag "version" | grep String
fetch.go:87:	f.StringVar(&fch.version, "version", "", "specific version of a chart. Without this, the latest version is fetched")
inspect.go:135:	inspectCommand.Flags().StringVar(&insp.version, verflag, "", verdesc)
inspect.go:136:	valuesSubCmd.Flags().StringVar(&insp.version, verflag, "", verdesc)
inspect.go:137:	chartSubCmd.Flags().StringVar(&insp.version, verflag, "", verdesc)
install.go:173:	f.StringVar(&inst.version, "version", "", "specify the exact chart version to install. If this is not specified, the latest version is installed")
package.go:98:	f.StringVar(&pkg.version, "version", "", "set the version on the chart to this semver version")
upgrade.go:114:	f.StringVar(&upgrade.version, "version", "", "specify the exact chart version to use. If this is not specified, the latest version is used")

We chose not to use -v as a short notation because so many other programs use -v to mean verbose. If we collectively think using -v for version is a good idea, we should change it in all the above places too.

[larryrensing]

I agree with the updating an existing dependency - i'll add that. With regards to the default version, I chose 0.1.0 because it is the smallest semver version. It can be changed to be any value, but i'd like to get some more input as to what it should be.

[wilkers-steve]

This looks great @larryrensing. Just one comment inline

[wilkers-steve]

Yeah, after some thought, I agree with you @thomastaylor312. Defaulting 'version' to a value that may not exist in the repository could result in unexpected behavior. I think latest does make sense here.

[technosophos]

Are we just waiting on the minor change to the --version flag at this point?

[prydonius]

Any thoughts on changing this to add instead of create?

[thomastaylor312]

I think add would be a clearer verb, but I wouldn't have a problem with create either

[technosophos]

add as a verb gets used in the helm repo * commands to indicate that a repository has been added to the index. That feels like the same sort of operation a user is doing here. So I agree.

[technosophos]

@larryrensing Any update on this?

[technosophos]

We're down to the wire on the 2.3.0 release, so I am going to bump this to 2.4.0.

[technosophos]

Any word on this one?

[technosophos]

Can we get a quick rebase of this before 2.4.0?

[technosophos]

Bumped to 2.5.0. Should I close this out? Is anyone still working on it?

[thomastaylor312]

@technosophos There hasn't been any motion on this for more than 30 days, so I am closing.

@larryrensing if you come back to this, feel free to reopen