Skip to content
This repository has been archived by the owner on Jun 29, 2022. It is now read-only.

Add path options to fact encrypt, decrypt, and redact #249

Merged
merged 2 commits into from
Aug 30, 2017
Merged

Conversation

noahl
Copy link

@noahl noahl commented Aug 30, 2017

Let users specify the path to write the encrypted, decrypted, or
redacted file to. If not given, default to not overwriting the
original report file.

Closes #247 .

Let users specify the path to write the encrypted, decrypted, or
redacted file to. If not given, default to not overwriting the
original report file.
@coveralls
Copy link

coveralls commented Aug 30, 2017

Coverage Status

Coverage decreased (-0.008%) to 68.927% when pulling 4919c83 on issues/247 into 9b5253d on master.

Copy link

@chambridge chambridge left a comment

Choose a reason for hiding this comment

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

A couple comments on the help wording. Additionally this information now needs to be added to the updated man documentation.


self.parser.add_option("--decrypted-path", dest="decrypted_path",
metavar="DECRYPTEDPATH",
help=_("Location for the decrypted file"))

Choose a reason for hiding this comment

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

Should use consistent working this is different than encryption help:
Destination for the encrypted file

Copy link

@elyezer elyezer Aug 30, 2017

Choose a reason for hiding this comment

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

I think all the new option could be named the same --path or --destinationfile or --somethingbetter. The subcommand would already provide the context if the user is encrypt, decrypt or redact.


self.parser.add_option("--redacted-path", dest="redacted_path",
metavar="REDACTEDPATH",
help=_("Path to write the redacted report to"))

Choose a reason for hiding this comment

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

Similar help consistency wording would be nice here.

@coveralls
Copy link

coveralls commented Aug 30, 2017

Coverage Status

Coverage decreased (-0.008%) to 68.927% when pulling 96993d3 on issues/247 into 9b5253d on master.

@chambridge chambridge merged commit abbc095 into master Aug 30, 2017
@chambridge chambridge deleted the issues/247 branch August 30, 2017 18:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants