rbt review <subcommand> --help?

Can you add a Version Added?

Can we give the base classes a Base prefix?

Some of the commands do some lookups and possible CommandError raising before operating on a draft. If we do this first, we can easily end up with empty drafts hanging around.
How about just moving this to a get_review_draft method or something, and calling it when ready to work on the draft?

We shouldn't need to provide this. It'll be the default (and will use the correct string type).

Typo: MARKDOMN -> MARKDOWN

Can we end all the help_text strings with a period, like we do in Command.description and help= strings?
Same applies below.

We use uppercase metavars in all options in the codebase. Can you update all the ones in the change to be uppercase?
(Should also be one word: "FILENAME")

This isn't needed.

This should probably use default=1. I think we can then avoid checking against None below.

Typo: "specifieud"

We do self.options all over the place. Let's pull it out into a local variable, reduce the noise and the repeated attribute lookups.
Same with api_root.
This applies to other implementations as well.

"fid" is kind of confusing. Can we just make it "ID"?

"id" -> "ID"
I don't think we need the quotes around the ID numbers. We don't use them elsewhere.

Missing a trailing period.

Typo: MARKDOMN -> MARKDOWN.
I state in another comment that we may want to consider a standard option we can plug in for Markdown, and given the error here, maybe this is a good time to do it?

I was looking into whether we could avoid having to use two variables and the complex logic below but also gain the useful error message about mutual-exclusivity.
argparse has an add_mutually_exclusive_group for this purpose, but it's probably too heavy-weight, and we don't have an Option/OptionGroup equivalent for it (though we could add it).
Anyway, so I checked other commands, and this scenario is all over the place, and probably isn't a big deal. A --thing and --no-thing both populating the same variable is common (such as rbt land --push and --no-push), and we haven't heard of any confusion around them yet.
So I think it'd be better to err on the side of simplicity here, have both these options use the same variable and default=None (which is the default value for default anyway), and then trust the value when making decisions below.
If we want to add mutual-exclusive support during parsing, we should probably handle that in common code somehow.

Missing a trailing period.

We have this logic three times in two existing commands, and are adding more.
While --markdown isn't a standard argument (we should probably make a constant for it though), I think it'd be valuable to have at least a utility function that gives us a text type for a given value + capability, and just use that.

The trailing newline isn't needed here.

Missing a trailing period.

Can we call this create_review_if_missing, and document it?

This can use the new helper function, right?

Ship It!