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

Move extraction of encryption and rotation options to separate functions #1387

Closed
wants to merge 4 commits into from

Conversation

felixfontein
Copy link
Contributor

Preparation for a rewrite of #1343, which aims at implementing #1333 (Make encrypt, decrypt, rotate, set, and edit proper commands).

Currently contains a rebased version of #974, since that adds more encryption options.

This simplifies the main command handler to a more manageable ~180 lines of code, and heavily reduces the copy'n'paste that would happen in an implementation like #1343.

mitar and others added 3 commits December 26, 2023 08:55
Signed-off-by: Mitar <mitar.git@tnode.com>
…Shamir threshold, etc.).

Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
@felixfontein
Copy link
Contributor Author

An unintended side-effect: if before, --encrypt --set has been specified together, the --encrypt fails (since the file is already encrypted), but the error returned by it is ignored by the old code and overwritten by --set's error (which is nil). 4e8796e restores the old behavior. (I'll create a PR to issue a warning if such commands are combined.)

Signed-off-by: Felix Fontein <felix@fontein.de>
@felixfontein
Copy link
Contributor Author

I modified that commit to 05ff087: first, --decrypt and --rotate have the same effect as --set on --encrypt (see #1388 which adds a warning of more than one command is used). Also the change for --rotate was already introduced in #1317 (and is already released), so I don't want to undo that behavior change. (It was fixing something even more broken.)

@felixfontein
Copy link
Contributor Author

#1389 is this PR without #974, so it can be reviewed and merged sooner.

@felixfontein
Copy link
Contributor Author

I moved the main commit to #1392, rebased upon #1389. I'm closing this one in favor of #1392 (and of course #1389).

@felixfontein felixfontein deleted the refactor-incl-974 branch December 27, 2023 16:45
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