Hello,

On Fri 08 Nov 2019 at 02:10AM -05, Daniel Kahn Gillmor wrote:

> This change has no functional change, it just makes it clear that
> there is a distinct condition for even trying to decrypt.

Er, are you sure about this?  There is a new sanity check and warning
message that might be output...

> +            if self.args.pgpkey:
> +                if pgpy is None:
> +                    logging.warning(f'Python module pgpy is not available, 
> not attempting to decrypt with --pgpkey arguments {self.args.pgpkey} (try 
> "apt install python3-pgpy")')

I'm not really a python programmer, so please just say if what you're
doing here is idiomatic, but it does strike me as inelegant to have to
indent all the remaining code into an 'else' block just to check for the
presence of a module.

How about factoring that code out into a function, which only gets
called if it was possible to load the module?

I think I'd find that easier to read, myself, but you're the one likely
to be maintaining this script, so I'll defer to you.

-- 
Sean Whitton

Attachment: signature.asc
Description: PGP signature

Reply via email to