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

Issue 6856120: environs/config: more tests for default-series

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 1 month ago by dfc
Modified:
5 years, 1 month ago
Reviewers:
mp+137118, niemeyer
Visibility:
Public.

Description

environs/config: more tests for default-series Laying the foundation for cross series bootstrapping. https://code.launchpad.net/~dave-cheney/juju-core/057-environs-config-default-series-tests/+merge/137118 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 1

Patch Set 2 : environs/config: more tests for default-series #

Patch Set 3 : environs/config: more tests for default-series #

Patch Set 4 : environs/config: more tests for default-series #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+20 lines, -5 lines) Patch
A [revision details] View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M environs/config/config_test.go View 1 4 chunks +18 lines, -5 lines 1 comment Download

Messages

Total messages: 8
dfc
Please take a look.
5 years, 1 month ago (2012-11-30 07:33:48 UTC) #1
niemeyer
LGTM, with a minor detail: https://codereview.appspot.com/6856120/diff/1/environs/config/config_test.go File environs/config/config_test.go (right): https://codereview.appspot.com/6856120/diff/1/environs/config/config_test.go#newcode55 environs/config/config_test.go:55: series: version.Current.Series, The "if" ...
5 years, 1 month ago (2012-11-30 17:19:56 UTC) #2
dfc
Please take a look.
5 years, 1 month ago (2012-12-03 02:08:50 UTC) #3
dfc
Please take a look.
5 years, 1 month ago (2012-12-03 02:10:56 UTC) #4
dfc
Please take a look.
5 years, 1 month ago (2012-12-03 02:20:34 UTC) #5
niemeyer
https://codereview.appspot.com/6856120/diff/8004/environs/config/config_test.go File environs/config/config_test.go (right): https://codereview.appspot.com/6856120/diff/8004/environs/config/config_test.go#newcode447 environs/config/config_test.go:447: if test.series != "" { This can be de-dented: ...
5 years, 1 month ago (2012-12-03 21:50:59 UTC) #6
dfc
I'm still struggling with testing CL 6851081. My hope was that by adding tests here ...
5 years, 1 month ago (2012-12-03 21:56:59 UTC) #7
niemeyer
5 years, 1 month ago (2012-12-03 22:04:09 UTC) #8
I've sent a reply to your comment on the list, with a potential suggestion. Not
quite sure if it'll be helpful. Please let me know either way.
Sign in to reply to this message.

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