Summary

Let user specify search path for .reviewboardrc

Review Request #7679 — Created Oct. 6, 2015 and submitted Oct. 20, 2015, 3:17 p.m.

Information

Owner

nilsph

Repository

RBTools

Branch

master

Bugs

Depends On

Commit

6c1524f...

Reviewers

Groups

rbtools

People

Description

If a user sets $RBTOOLS_CONFIG_PATH, it should contain a list of paths
separated by the characters conventionally used for this on the
operating system (":" on Linux, Mac OS X, ";" on Windows). These paths
are searched first for .reviewboardrc and can be used to override
repository-wide settings.

Reviewed at https://reviews.reviewboard.org/r/7679/

Testing Done

Ran RBTOOLS_CONFIG_PATH=$HOME rbt status in a project directory with REVIEWBOARD_URL set to http://127.0.0.1 in $HOME/.reviewboardrc, got:
 ERROR: Unexpected API Error: HTTP 404

Confirmed that it picks up the private .reviewboardrc first.

Issues

Description	From	Last Updated
:file:`.reviewboardrc`	brennie	Oct. 9, 2015, 2:01 a.m.

Tool: PEP8 Style Checker
Processed Files:
    rbtools/utils/filesystem.py



Tool: Pyflakes
Processed Files:
    rbtools/utils/filesystem.py

```
Minor nit.
```

rbtools/utils/filesystem.py (Diff revision 1)

Show all issues

:file:`.reviewboardrc`

nilsph Oct. 9, 2015, 2:05 a.m.

Just to fix the minor nit, not to forgo the question brought up by David.

This is going to be very controversial. We had initially done this when we moved from post-review to rbt, and we had a number of complaints. I'd like to see some discussion of this on the Review Board mailing list before we accept this change.

nilsph Oct. 9, 2015, 2:05 a.m.

I understand. Would optionally (and then preferentially) reading a separate, clearly named file (e.g. $HOME/.reviewboard-overrides) be less controversial?

chipx86 Oct. 9, 2015, 2:12 a.m.

I think that'd end up causing some additional confusion as to which file people should put things in.
Here's an idea for discussion: How about an environment variable, $RBTOOLS_CONFIG_PATH, that defines where to look for .reviewboardrc, prior to the built-in checks? So, it's prepended to the list of configs we search by default (and if any built-in searched directory exists in the path, we just skip it).
That would let you do:
export RBTOOLS_CONFIG_PATH=$HOME

Then that would be searched first, prior to any of the other directories. Gives the control you need, without breaking existing setups.

nilsph Oct. 9, 2015, 4:10 a.m.
```
Sounds reasonable to me. David?
```
david Oct. 9, 2015, 4:35 p.m.
```
Yeah, seems fine.
```

Commit:

-	6ea0ffb2cb9d85c6f47af7eba476482151306f29
+	6c1524f91223aef40f319d828a5e456d95fdea34

Diff:

Revision 2 (+13 -10)

Show changes

rbtools/utils/filesystem.py

Tool: PEP8 Style Checker
Processed Files:
    rbtools/utils/filesystem.py



Tool: Pyflakes
Processed Files:
    rbtools/utils/filesystem.py

Change Summary:

Implemented searching $RBTOOLS_CONFIG_PATH for .reviewboardrc rather than putting treating $HOME/.reviewboardrc by default.

Summary:

-	Ensure the reviewboardrc in HOME takes precedence
+	Let user specify search path for .reviewboardrc

Description:

~		This is important so users can e.g. set GUESS_FIELDS to a different
~		value than a non-default value set in a project. Setting bogus values
~		for actually project-specific settings (e.g. REVIEWBOARD_URL) will
~		obviously fail.
	~	If a user sets $RBTOOLS_CONFIG_PATH, it should contain a list of paths
	~	separated by the characters conventionally used for this on the
	~	operating system (":" on Linux, Mac OS X, ";" on Windows). These paths
	~	are searched first for .reviewboardrc and can be used to override
	+	repository-wide settings.

		Reviewed at https://reviews.reviewboard.org/r/7679/

Testing Done:

~		Ran `rbt status` in a project directory with `REVIEWBOARD_URL` set to `http://127.0.0.1` in `$HOME/.reviewboardrc`, got:
	~	Ran `RBTOOLS_CONFIG_PATH=$HOME rbt status` in a project directory with `REVIEWBOARD_URL` set to `http://127.0.0.1` in `$HOME/.reviewboardrc`, got:

		`ERROR: Unexpected API Error: HTTP 404`


~		Confirmed that it picks up the private `reviewboardrc` first.
	~	Confirmed that it picks up the private `.reviewboardrc` first.

Diff:

Revision 3 (+28 -3)

Show changes

	docs/rbtools/rbt/configuration/users.rst
	rbtools/utils/filesystem.py

Tool: Pyflakes
Processed Files:
    rbtools/utils/filesystem.py

Ignored Files:
    docs/rbtools/rbt/configuration/users.rst



Tool: PEP8 Style Checker
Processed Files:
    rbtools/utils/filesystem.py

Ignored Files:
    docs/rbtools/rbt/configuration/users.rst

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-0.7.x (8bda4a3)