Code review comment for lp:~dimitern/juju-core/260-lp-1067979-duplicate-charm
Revision history for this message
| Dimiter Naydenov (dimitern) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
Please take a look.
https:/
File state/apiserver
https:/
state/apiserver
(storage.Storage, error) {
On 2014/01/28 12:00:32, fwereade wrote:
> maybe put this in export_test rather than growing the interface here?
It was there, but I needed it in two places. I'll put it in
environs/testing.
https:/
File state/apiserver
https:/
state/apiserver
apiserver.
On 2014/01/28 12:00:32, fwereade wrote:
> ah, ok, you can't put it in export_test. Hmm. I think I'd maybe prefer
it if
> this were accessed via the environ on the test, but I'm not sure of
the
> tradeoffs -- I'll leave it to your judgment.
I'll put it in environs/testing.
https:/
File state/state.go (right):
https:/
state/state.go:480: what := D{{"_id", curl}, {"placeholder", false},
{"pendingupload", false}}
On 2014/01/28 12:00:32, fwereade wrote:
> Yeah, these bits do need to be fixed. Sorry, but they *will* break
stuff.
Done.
https:/
state/state.go:625: Insert: uploadedCharm,
On 2014/01/28 12:00:32, fwereade wrote:
> Don't we still need to deal with a placeholder here?
If we reach this point we know it's not a placeholder and there's no
document for this charm URL. Not setting Placeholder: false here works,
because it's the default value. So it's OK I think.
https:/
File utils/trivial.go (right):
https:/
utils/trivial.
error) {
On 2014/01/28 12:00:32, fwereade wrote:
> ReadSHA256?
Done.
https:/
utils/trivial.
int64, error) {
On 2014/01/28 12:00:32, fwereade wrote:
> ReadFileSHA256?
Done.
https:/
utils/trivial.
On 2014/01/28 12:00:32, fwereade wrote:
> Also, please test these. Not quite trivial enough to be skippable IMO.
Done.
https:/