Code review comment for lp://staging/~wallyworld/juju-core/image-metadata-defaults

Revision history for this message
John A Meinel (jameinel) wrote :

just going off of the description:

I really like having the settings unified. I'm not sure if the names
quite convey the intent. "override-image-id" meaning the one you use if
nothing can be found doesn't seem as good of a match (name wise) as
default-image-id.

"default" really sounds like "what to use if you can't find something
better".

Name wise, your (2) sounds more like "preferred-image-id" [if you find
multiple matches, prefer this one] and then (3) would be
"default-image-id" [if you can't find one, use this].
Though I'm not entirely sure why the use case for (3) isn't satisfied by
(2).

I think the concern was that you don't know if (2) actually matches the
request (because you only got the image-id, and not the metadata).

I would like us to think carefully if we really need to add one more
conf item.

If we do, I would probably like to see us leave default-image-id as the
one we use if we can't find one from metadata (3), and then add
something like "preferred-image-id" to indicate the way to break ties
(2).

Does that sound reasonable to you?

https://codereview.appspot.com/9303051/

« Back to merge proposal