Add --force-detach flag to bypass detached HEAD check#155
Add --force-detach flag to bypass detached HEAD check#155tummychow merged 5 commits intotummychow:masterfrom
Conversation
src/lib.rs
Outdated
| pub force_author: bool, | ||
| pub force_detach: bool, | ||
| // force should only be used to enable oher force_* values when unifiying the config. | ||
| // Do not access when disabling individual safety checks. |
There was a problem hiding this comment.
Commenting is a little weak. I figure this would be more robust if either there were a new kind of configuration-holding bag that config::unify built out of this Config and the git config (or alternatively it could build this object from main::Cli, but it seemed like kind of a large change to put in without consultation.
There was a problem hiding this comment.
i would say, remove force from Config entirely since it's just for the cli...
There was a problem hiding this comment.
Tidy. I generally like the idea, but it requires adjustments to the format of the tests, no? I'd been following along with the pattern of testing lib.rs and below.
Hmm. You've probably considered all this and are okay with the risk of not covering the new logic in main.rs. I'll make the simple change (all I have time for before going to work anyhow) and if you're not content, am happy to evolve the solution.
|
rebase please? this is very difficult to review with the other stuff mixed into it |
blairconrad
force-pushed
the
force-detach
branch
from
34d5ba2 to
f4165a7
Compare
Rebased, but can only get rid of commits from #153. I swear I'm not trying to make life difficult for you. If I hadn't encountered the surprising behaviour in the middle of this fix, it'd be quite a bit smoother. In the future if something like this comes up, I can hold off sending multiple PRs at once and just say that there's a current PR that in some way is a prerequisite for a future PR, or something. |
blairconrad
force-pushed
the
force-detach
branch
from
f4165a7 to
8cdcd92
Compare
|
Dropping PR #154, keeping some of the tests, and rebasing. Should be ready for review now. Thanks for your patience. |
A bonus change, discovered when I nearly use the wrong signature for detach_head
blairconrad
force-pushed
the
force-detach
branch
from
8cdcd92 to
b24341d
Compare
src/lib.rs
Outdated
| pub force_author: bool, | ||
| pub force_detach: bool, | ||
| // force should only be used to enable oher force_* values when unifiying the config. | ||
| // Do not access when disabling individual safety checks. |
There was a problem hiding this comment.
i would say, remove force from Config entirely since it's just for the cli...
src/main.rs
Outdated
| dry_run, | ||
| force_author, | ||
| force_detach, | ||
| force, |
There was a problem hiding this comment.
... and then down here, set force_detach: force_detach || force, same for force_author
Avoids the possibility of misusing the flag in lib and below, but now the unit tests do not test the use of the --force flag.
|
@tummychow, removed |
|
thanks. and yeah i basically don't care about testing the code in the cli entrypoint atm. there's not much actual logic there and i'm not a stickler for coverage |
2 participants
Fixes #145.
Also adds forceDetach configuration option.