Improve guess support in rbt post.
Review Request #5614 — Created March 12, 2014 and submitted
This adds new abilities to rbt post's guess fields support. The old
guess support was a boolean state, which only supported opt-in and not
opt-out. That is, if
GUESS_FIELDSwas set to
couldn't be turned off when posting.
It's now a tri-state of
"no", and "
auto", which can be set on the
command line or
"yes", it behaves as before, with the fields being updated from the
"no", guessing will be turned off (which allows the command line to
override the config file).
"auto", guessing will be turned on only for new review requests, but
turned off when updating.
--guess-*options or configuration variables are set, the
default will be
"auto", making for a nicer experience when posting brand
new review requests.
If an old-style
--guess-*option is passed without a value (in the form
-gvalue), then the prior logic of forcing guessing
will be used. That is,
"auto"are all valid values, we still support
True and False in
.reviewboardrc, for backwards-compatibility.
Posted this change by doing
rbt post, without any parameters.
Temporarily turned off actual publishing of the change and just outputted
the results of the guess flags. Then I did tests with all three values
.reviewboardrcand with/without command line arguments and their possible
values. Saw the resulting values that I expected.
Turned posting back on, removed the settings from
.reviewboardrc, and posted
a change without explicitly passing any guess arguments. The review request
had the fields filled in from my commit message.
Made changes to the description and summary on my review request, and updated
-r. The changes weren't overwritten. Tested also with
Then I tried with
--guess-fields=no, and it didn't overwrite either.
yesand it did overwrite.
Tried again without a value and it overwrote again.
Posted a new change with
--guess-fields=noand it didn't provide any defaults.
Unit tests pass.
Built the docs, read through, and verified all links.
This should probably mention that it only applies to SCMs where you're posting commits (git, hg, bzr).
I'd prefer to not include this example.
How about printing the value as well?
Maybe 'Invalid value "%s" for argument "%s"' ?
How about "changes from a working directory"? "staged" is only really git terminology.
- Mentioned that guessing only works for commit-based repos.
- Removed the example for short-form args.
- Printed the value in the error when normalizing guess values fails.
Revision 2 (+202 -33)
This --guess flag behavior is one facet of the larger issue of config inheritance from ~/.reviewboardrc, $REPO/.reviewboardrc down through any command line options supplied. At least elsewhere, a common way to handle this is set the cli flag defaults to the values inhereited from the config file.
To me, it seems you've replicated this behavior except using a tristate enum. This pattern gets a bit wierder when extended to non boolean values. Consider the inheritiance of the --repository option. All of a sudden you're taking arbitray string values except 'auto' is special cased, and a repository can't be named 'auto' without breaking things (far-fetched... but still).