Merge tools metadata when writing

Merge tools metadata when writing

If there is already tools metadata present when writing new
metadata, do not overwrite it. Instead, merge any new tools
records with what's there.

https://code.launchpad.net/~wallyworld/juju-core/simplestreams-metadata-merge/+merge/185402

(do not edit description out of merge proposal)

[wallyworld, 2013-09-13 02:05:55]

Please take a look.

[thumper, 2013-09-13 02:35:31]

LGTM - would like a more meaningful name, but it isn't a blocking request.

https://codereview.appspot.com/13274045/diff/1/environs/tools/simplestreams.go
File environs/tools/simplestreams.go (right):

https://codereview.appspot.com/13274045/diff/1/environs/tools/simplestreams.g...
environs/tools/simplestreams.go:186: func WriteMetadata(toolsList
coretools.List, fetch bool, stor environs.Storage) error {
what is "stor" storing? Can we give it a more meaningful name?

https://codereview.appspot.com/13274045/diff/1/environs/tools/simplestreams.g...
environs/tools/simplestreams.go:194: existingMetadata, err :=
Fetch([]simplestreams.DataSource{dataSource}, simplestreams.DefaultIndexPath,
toolsConstraint, false)
I really hate boolean parameters. There is nothing in this context that tells me
what "false" means.

[wallyworld, 2013-09-13 02:41:01]

https://codereview.appspot.com/13274045/diff/1/environs/tools/simplestreams.go
File environs/tools/simplestreams.go (right):

https://codereview.appspot.com/13274045/diff/1/environs/tools/simplestreams.g...
environs/tools/simplestreams.go:186: func WriteMetadata(toolsList
coretools.List, fetch bool, stor environs.Storage) error {
On 2013/09/13 02:35:31, thumper wrote:
> what is "stor" storing? Can we give it a more meaningful name?

It's an environs stor directly from the environs instance. In practice, it's the
control bucket. But I didn't want to expose that here. I've renamed to
metadataStore.

https://codereview.appspot.com/13274045/diff/1/environs/tools/simplestreams.g...
environs/tools/simplestreams.go:194: existingMetadata, err :=
Fetch([]simplestreams.DataSource{dataSource}, simplestreams.DefaultIndexPath,
toolsConstraint, false)
On 2013/09/13 02:35:31, thumper wrote:
> I really hate boolean parameters. There is nothing in this context that tells
me
> what "false" means.

Yep. Too bad Go doesn't have named parameters.

[axw1, 2013-09-13 03:28:09]

On 2013/09/13 02:41:01, wallyworld wrote:
>
https://codereview.appspot.com/13274045/diff/1/environs/tools/simplestreams.g...
> environs/tools/simplestreams.go:194: existingMetadata, err :=
> Fetch([]simplestreams.DataSource{dataSource}, simplestreams.DefaultIndexPath,
> toolsConstraint, false)
> On 2013/09/13 02:35:31, thumper wrote:
> > I really hate boolean parameters. There is nothing in this context that
tells
> me
> > what "false" means.
> 
> Yep. Too bad Go doesn't have named parameters.

An alternative is:

type FetchFlag bool

const (
    NoFetch FetchFlag = false
    Fetch FetchFlag = true
)