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

Fix Export typos #305

Merged
merged 2 commits into from
Jun 20, 2018
Merged

Fix Export typos #305

merged 2 commits into from
Jun 20, 2018

Conversation

redhatrises
Copy link
Collaborator

  • typos were causing crashes

- typos were causing crashes
@codecov
Copy link

codecov bot commented Jun 19, 2018

Codecov Report

Merging #305 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #305   +/-   ##
=======================================
  Coverage   79.25%   79.25%           
=======================================
  Files          38       38           
  Lines        1427     1427           
=======================================
  Hits         1131     1131           
  Misses        244      244           
  Partials       52       52
Impacted Files Coverage Δ
pkg/cli/export/export.go 0% <0%> (ø) ⬆️

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 43c0366...49d69fb. Read the comment docs.

@@ -49,12 +49,12 @@ func RunExport(out io.Writer, cmd *cobra.Command, args []string) error {
}

// read parms
parmOpencontrols := cmd.Flag("opencontrols").Value.String()
parmDestination := cmd.Flag("destination").Value.String()
parmOpencontrols := cmd.Flag("opencontrol").Value.String()
Copy link
Member

Choose a reason for hiding this comment

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

should this be renamed to parmOpencontrol to match?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@shawndwells doesn't have to be, but can if you want it.

Copy link
Member

Choose a reason for hiding this comment

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

The naming should be aligned.

Was looking at how it's used:

	// construct args
	config := Config{
		Certification:   args[0],
		OpencontrolDir:  parmOpencontrols,
		DestinationFile: parmDestination,
		OutputFormat:    outputFormat,
		Flatten:         parmFlatten,
		InferKeys:       parmInferKeys,
		Docxtemplater:   parmDocxtemplater,
		KeySeparator:    parmKeySeparator,
	}

Should it really be called something like parmInputDir?

Minimally should be parmOpencontrol (singular) to match the command flag. parmInputDir would be more descriptive.

What do you think?

Not the most important thing in the world, but while this part of the code is being touched....

@shawndwells shawndwells merged commit 8f11f13 into opencontrol:master Jun 20, 2018
@redhatrises redhatrises deleted the fix_export branch June 20, 2018 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants