Rename arguments

[tomaszkiewicz]

This pull request renames arguments to be compatible with styling of docker and Go commands.
Also fixes a couple of typos.

[mdlavin]

Thanks for the PR! I love the spelling fixes and the consistency with Go conventions. To avoid breaking current users, would it be possible to support the old argument names too? They don't need to show up in the help documentation, but it would be nice if a new version didn't break anybody.

[tomaszkiewicz]

I'm afraid it would be pretty hard to accomplish that, as kingpin.Flag code that defines flag names is executed before parsing, so I'm unable to "fallback" in buildRegistryArguments as flags are not parsed yet.

My second try was to use the same pointer for two flag names, like that:

func buildRegistryArguments(argPrefix string, argDescription string) RepositoryArguments {
	var registryURLArg, repositoryArg, tagArg *string

	getMultipleFlagsWithPrefix(registryURLArg, []string{"%s-url", "%sURL"}, "URL of %s registry", argPrefix, argDescription)
	getMultipleFlagsWithPrefix(repositoryArg, []string{"%s-repo", "%sRepo"}, "Name of %s repository", argPrefix, argDescription)
	getMultipleFlagsWithPrefix(tagArg, []string{"%s-tag", "%sTag"}, "Name of the %s tag", argPrefix, argDescription)

	return RepositoryArguments{
		RegistryURL: registryURLArg,
		Repository:  repositoryArg,
		Tag:         tagArg,
	}
}

func getMultipleFlagsWithPrefix(value *string, flagTemplates []string, descriptionTemplate string, flagPrefix string, argDescription string) {
	for _, flagTemplate := range flagTemplates {
		name := fmt.Sprintf(flagTemplate, flagPrefix)
		description := fmt.Sprintf(descriptionTemplate, argDescription)
		kingpin.Flag(name, description).StringVar(value)
	}
}

But unfortunately it causes program to crash for unknown reason... Tried to debug that but without success :(

Any ideas on how to implement that?

[mdlavin]

Thanks for attempting to make it work. It doesn't jump out to my why it would crash and it's probably not worth the trouble. On re-review i noticed that there are still some references to the old style of argument in the README. Can you update that file to match (and maybe search the code for other lingering references)?

[tomaszkiewicz]

Done :)

BTW: I've extended the project a little on my fork (https://github.com/tomaszkiewicz/docker-copy-docker-image) by adding Dockerfile (based on alpine instead of scratch, so can be used on Jenkins) and changing some ECR logic so it doesn't require AWS config file with region specified (it uses url and gets region from ECR url - very convenient when used in container).
Also I've fixed an issue with manifests when copying from Docker Hub to ECR (ECR requires name in manifest to match the one in repo name) - you can take a look and I can add PR for all above changes if you like them :)

[mdlavin]

I'm happy to merge any PRs that make the tool more usable. Both of your ideas sound useful to me. If you open PRs I'm happy to review and get them merged