Code review comment for lp://staging/~tealeg/landscape-client/allow-duplicate-keys-in-config

Revision history for this message
Geoff Teale (tealeg) wrote :

Cheers for the review Chris.

> [1]

Done - parsing continues after the failure so the later config values are read correctly

> [2]
> What happens when more than one keys are duplicates? triplicates?

I've added a test demonstrating this - what you get is a compound error message, but otherwise the number of repetitions doesn't change the behaviour (We always get the first value).

>
> [3]
> + but the latest defined value should be used.
> + """
>
> This contradicts the test, where the *first* defined value is used.

Yup, good spot - I wrote the docstring first ;-) Fixed

> [4]
> + config_obj = ConfigObj(config_source, list_values=False,
> + raise_errors=False)
>
> Please add a comment explaining what raise_errors does (similar to the one
> about list_values)

Done.

« Back to merge proposal