Summary

Improve guess support in rbt post.

Review Request #5614 — Created March 12, 2014 and submitted March 14, 2014, 5:45 p.m.

Information

Owner

chipx86

Repository

RBTools

Branch

master

Bugs

Depends On

Commit

ee401fd...

Reviewers

Groups

rbtools

People

Description

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_FIELDS was set to True in .reviewboardrc, it
couldn't be turned off when posting.

It's now a tri-state of "yes", "no", and "auto", which can be set on the
command line or .reviewboardrc.

If "yes", it behaves as before, with the fields being updated from the
commit message.

If "no", guessing will be turned off (which allows the command line to
override the config file).

If "auto", guessing will be turned on only for new review requests, but
turned off when updating.

If no --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
of --guess-*=value or -gvalue), then the prior logic of forcing guessing
will be used. That is, "yes" is implied.

While "yes", "no", and "auto" are all valid values, we still support
True and False in .reviewboardrc, for backwards-compatibility.

Testing Done

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
in .reviewboardrc and 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
using -r. The changes weren't overwritten. Tested also with -u.

Then I tried with --guess-fields=no, and it didn't overwrite either.

Tried with yes and it did overwrite.

Tried again without a value and it overwrote again.

Posted a new change with --guess-fields=no and it didn't provide any defaults.

Unit tests pass.

Built the docs, read through, and verified all links.

Issues

Description	From	Last Updated
This should probably mention that it only applies to SCMs where you're posting commits (git, hg, bzr).	david	March 14, 2014, 5:22 p.m.
I'd prefer to not include this example.	david	March 14, 2014, 5:22 p.m.
How about printing the value as well?	david	March 14, 2014, 5:23 p.m.
Maybe 'Invalid value "%s" for argument "%s"' ?	david	March 14, 2014, 5:44 p.m.
How about "changes from a working directory"? "staged" is only really git terminology.	david	March 14, 2014, 5:45 p.m.

docs/rbtools/rbt/commands/post.txt (Diff revision 1)

The issue has been resolved. Show all issues

This should probably mention that it only applies to SCMs where you're posting commits (git, hg, bzr).

docs/rbtools/rbt/commands/post.txt (Diff revision 1)

The issue has been resolved. Show all issues

I'd prefer to not include this example.

chipx86 March 12, 2014, 4:46 p.m.

I could be more explicit about it, but it's actually a useful one if you want to disable guessing entirely for this post.

Was it just the short form you didn't like?

david March 12, 2014, 5:02 p.m.
```
Yeah, just the short form.
```

rbtools/commands/post.py (Diff revision 1)
The issue has been resolved. Show all issues
```
How about printing the value as well?
```

Change Summary:


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.

Commit:

2aedab0bdfa29fb8507714aa06d64c782af15f6d

ee401fdc19f7d0511fd9b510785b60910e2e19d8

Diff:

Revision 2 (+202 -33)

Show changes

	docs/rbtools/rbt/commands/post.txt
	rbtools/commands/__init__.py
	rbtools/commands/post.py

Ship it!

rbtools/commands/post.py (Diff revisions 1 - 2)
The issue has been resolved. Show all issues
```
Maybe 'Invalid value "%s" for argument "%s"' ?
```

docs/rbtools/rbt/commands/post.txt (Diff revision 2)

The issue has been resolved. Show all issues

How about "changes from a working directory"? "staged" is only really git terminology.

Status:: Completed

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.

https://stackoverflow.com/questions/3609852

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).