Page MenuHomePhabricator

strip: move into core
ClosedPublic

Authored by valentin.gatienbaron on Sun, Nov 8, 6:30 PM.

Details

Summary

As discussed at the 5.2 sprint, replace strip extension by a core
command, debugstrip. Obviously, the extension stays for backwards
compatibility.

As an implementation note, I moved the strip file as is into core,
which is not done elsewhere, AFAIK. I could have inlined it into
debugcommands, but that doesn't sound great.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

My memory from the discussion was that we explicitly wanted to keep it as extension as it normal users shouldn't use this and it is considered dangerous?

Was that the conclusion? I think I was taking notes and missed it. I thought turning the strip extension into a debugstrip command was already agreed upon a year ago, In fact, D6987 was started back then (and stalled, but that's unrelated).
There's no disagreement that the command is dangerous, that's why it's debugstrip and not strip. That seems like better pushback to me than an extension.

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.

This fails test-check-format.t for me. I'll fix it in flight.

Oops, sorry, I didn't have black installed, so test-check-format.t was getting skipped. Having installed it now (at version 19.10, because 20.8b is broken), there are indeed formatting issues. Thanks for the fix.