Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(5521)

Issue 94530045: Fix arch preference algorithm

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 11 months ago by axw
Modified:
9 years, 11 months ago
Reviewers:
mp+219839, fwereade, wallyworld
Visibility:
Public.

Description

Fix arch preference algorithm Change the instance architecture preference selection to widest word-size, then first arch name by alphabetical order. For tie- breakers, choose the instance type based on provider-specific ordering. In the future we may want to extend the ordering to include cost. For now, we rely on the providers ordering instance types by their own preference (no change here). https://code.launchpad.net/~axwalk/juju-core/instances-arch-preference/+merge/219839 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 3

Patch Set 2 : Fix arch preference algorithm #

Unified diffs Side-by-side diffs Delta from patch set Stats (+162 lines, -66 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M environs/instances/image.go View 4 chunks +44 lines, -19 lines 0 comments Download
M environs/instances/image_test.go View 1 8 chunks +94 lines, -47 lines 0 comments Download
M juju/arch/arch.go View 1 chunk +15 lines, -0 lines 0 comments Download
M juju/arch/arch_test.go View 1 chunk +7 lines, -0 lines 0 comments Download

Messages

Total messages: 4
axw
Please take a look.
9 years, 11 months ago (2014-05-16 13:10:42 UTC) #1
fwereade
I feel like we're missing some tests here. Otherwise looking good. https://codereview.appspot.com/94530045/diff/1/environs/instances/image_test.go File environs/instances/image_test.go (right): ...
9 years, 11 months ago (2014-05-16 21:12:38 UTC) #2
axw
Please take a look. https://codereview.appspot.com/94530045/diff/1/environs/instances/image_test.go File environs/instances/image_test.go (right): https://codereview.appspot.com/94530045/diff/1/environs/instances/image_test.go#newcode194 environs/instances/image_test.go:194: }, On 2014/05/16 21:12:38, fwereade ...
9 years, 11 months ago (2014-05-19 03:33:53 UTC) #3
wallyworld
9 years, 11 months ago (2014-05-19 07:16:28 UTC) #4
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b