Code review comment for lp:~jelmer/bzr-pqm/2.5-compat

On 01/20/2012 09:42 AM, Vincent Ladeuil wrote:
> Review: Needs Fixing
>
> You beat me to it ;)
>
> 8 +
>
> Spurious.
>
> 16 + get = _get_user_option
> 17 +
>
> Cunning !
>
> 42 + if bzrlib_version< (2, 5, 0, 'dev', 5):
>
> (2,5,0,'beta',5)< (2, 5, 0, 'dev', 5)
> True
>
> Oops !
>
> I think we should go with just (2,5) instead, people encountering issues
> with 2.5 compatibility are likely to 1) use at least 2.5b5 or trunk 2) upgrade to at least 2.5b5 anyway.
Fair enough.
>
>
> 51 + sections = []
> 52 + if branch:
> 53 + bstore = branch._get_config_store()
> 54 + sections.append(_mod_config.NameMatcher(bstore, None).get_sections)
> 55 if public_location:
> 56 - config.add_source(_mod_config.LocationConfig(public_location))
> 57 - config.add_source(_mod_config.GlobalConfig())
> 58 + lstore = _mod_config.LocationStore()
> 59 + sections.append(_mod_config.LocationMatcher(lstore,
> 60 + public_location).get_sections)
> 61 + gstore = _mod_config.GlobalStore()
> 62 + sections.append(_mod_config.NameMatcher(gstore, 'DEFAULT').get_sections)
> 63 + config = _mod_config.Stack(sections)
>
> This doesn't look correct (locations.conf should override branch,conf when
> 'branch' is not None and -O also needs to supported), I think we want:
It's consistent with the behaviour in the pre-2.5 version of bzr-pqm. I
don't think we should change that.

Cheers,

Jelmer