Summary

Ensure the reviewboardrc in HOME is only parsed once

Review Request #7232 — Created April 21, 2015 and submitted April 21, 2015, 5:35 p.m.

Information

Owner

brennie

Repository

RBTools

Branch

release-0.7.x

Bugs

Depends On

Blocks

7234

Commit

ab49e4f...

Reviewers

Groups

rbtools

People

Description

Previously, the list of configuration paths for get_config_paths
could have duplicates in the case where the current working directory
is a subdirectory of the home directory and a configuration file exists
in the home directory. This will lead to the config in the home
directory being parsed twice, which is undesirable.

Also the documentation in get_config_paths was incorrect as to the
order of precedence in the returned list of paths. This has been
corrected.

Testing Done

Ran unit tests.

Posted this review request.

Checked the output of get_config_paths before and after applying this
patch. Before applying this patch, my ~/.reviewboardrc appears in the
list of config paths twice. After applying this patch, it only appears
once and is the last element of the list.

Manually verified the result of load_config to ensure that my
./reviewboardrc was only parsed once.

Issues

Description	From	Last Updated
'OrderedDict' imported but unused	reviewbot	April 21, 2015, 7:47 a.m.
'six' imported but unused	reviewbot	April 21, 2015, 7:47 a.m.
Actually this isn't true. We want the repository-specific one to have the highest precedence.	david	April 21, 2015, 3:40 p.m.

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



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

rbtools/utils/filesystem.py (Diff revision 1)
Show all issues
```
 'OrderedDict' imported but unused
```
rbtools/utils/filesystem.py (Diff revision 1)
Show all issues
```
 'six' imported but unused
```

Change Summary:

Remove unused imports.

Commit:

-	0efa70fcc11f1c5b04cd2e6a5d5e399b00b883cc
+	5c646d1a7f71ff2e9c2c2dbe12ce41f5f4d9b618

Diff:

Revision 2 (+11 -5)

Show changes

rbtools/utils/filesystem.py

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



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

Summary:

-	Only parse the .reviewboardrc file in HOME once
+	Parse configuration files in correct order

Description:

		Previously, the list of configuration paths for `get_config_paths`
		could have duplicates in the case where the current working directory
		is a subdirectory of the home directory and a configuration file exists
		in the home directory. This will lead to the config in the home
		directory being parsed twice, which is undesirable.

~		This patch ensures that the configuration in the home directory only
~		appears once in the list of configuration paths.
	~	Also, configuration files were being parsed in an incorrect order. The
	~	documentation for `get_config_paths` indicated that items occuring
	+	later in the list had higher precedence, but we were parsing them in a
	+	reverse order. This caused configuration keys in higher precedence
	+	files being overwritten by configuration keys in lower precedence
	+	files.
	+
	+	This patch addresses both issues.

Testing Done:

		Ran unit tests.

		Posted this review request.

		Checked the output of `get_config_paths` before and after applying this
		patch. Before applying this patch, my `~/.reviewboardrc` appears in the
		list of config paths twice. After applying this patch, it only appears
		once and is the last element of the list.
	+
	+	Manually verified the result of `load_config` to ensure that my
	+	`./reviewboardrc` file had the highest precedence.

Commit:

-	5c646d1a7f71ff2e9c2c2dbe12ce41f5f4d9b618
+	81733e195a82f343a09e80a01c94d16d45bf7d19

Diff:

Revision 3 (+14 -12)

Show changes

	.reviewboardrc
	rbtools/utils/filesystem.py

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

Ignored Files:
    .reviewboardrc



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

Ignored Files:
    .reviewboardrc

Change Summary:

Undo changes to .reviewboardrc from testing.

Commit:

-	81733e195a82f343a09e80a01c94d16d45bf7d19
+	85c40c41bfb703fd8d118febebbadcd246bdc1a5

Diff:

Revision 4 (+13 -12)

Show changes

rbtools/utils/filesystem.py

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



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

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

Show all issues

Actually this isn't true. We want the repository-specific one to have the highest precedence.

Summary:

-	Parse configuration files in correct order
+	Ensure the reviewboardrc in HOME is only parsed once

Description:

		Previously, the list of configuration paths for `get_config_paths`
		could have duplicates in the case where the current working directory
		is a subdirectory of the home directory and a configuration file exists
		in the home directory. This will lead to the config in the home
		directory being parsed twice, which is undesirable.

~		Also, configuration files were being parsed in an incorrect order. The
~		documentation for `get_config_paths` indicated that items occuring
~		later in the list had higher precedence, but we were parsing them in a
	~	Also the documentation in `get_config_paths` was incorrect as to the
	~	order of precedence in the returned list of paths. This has been
	~	corrected.
-		reverse order. This caused configuration keys in higher precedence
-		files being overwritten by configuration keys in lower precedence
-		files.
-
-		This patch addresses both issues.

Testing Done:

		Ran unit tests.

		Posted this review request.

		Checked the output of `get_config_paths` before and after applying this
		patch. Before applying this patch, my `~/.reviewboardrc` appears in the
		list of config paths twice. After applying this patch, it only appears
		once and is the last element of the list.

		Manually verified the result of `load_config` to ensure that my
~		`./reviewboardrc` file had the highest precedence.
	~	`./reviewboardrc` was only parsed once.

Commit:

-	85c40c41bfb703fd8d118febebbadcd246bdc1a5
+	ab49e4f1dfd937a3b6077ea23113d2204f00005b

Diff:

Revision 5 (+11 -7)

Show changes

rbtools/utils/filesystem.py

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



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

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-0.7.x (d6f8822)