Add support for resources. #19

Merged
merged 7 commits into from Mar 16, 2016

Conversation

Projects
None yet
5 participants
Contributor

ericsnowcurrently commented Mar 15, 2016

No description provided.

Member

jujugui commented Mar 15, 2016

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
http://ci.jujugui.org:8080//job/bundlechanges/52/
Test FAILed.

changes.go
+ if resources == nil {
+ resources = make(map[string]int, 0)
+ }
+ return []interface{}{ch.Params.Charm, ch.Params.Service, options, ch.Params.Constraints, storage, endpointBindings, resources}
@frankban

frankban Mar 16, 2016

Member

The GUI should be ready to accept the new resources argument before adding them here.
I'd say let's leave GUIArgs untouched for now.

@frankban

frankban Mar 16, 2016

Member

Maybe add a TODO comment?

@ericsnowcurrently

ericsnowcurrently Mar 16, 2016

Contributor

Sounds good. I'm glad you noticed this. It was the one thing I was unsure about. :)

Member

frankban commented Mar 16, 2016

👍 with a change.
Thank you!

Member

jujugui commented Mar 16, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://ci.jujugui.org:8080//job/bundlechanges/53/
Test PASSed.

Member

jujugui commented Mar 16, 2016

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
http://ci.jujugui.org:8080//job/bundlechanges/54/
Test PASSed.

👍

kat-co commented Mar 16, 2016

+1

Contributor

ericsnowcurrently commented Mar 16, 2016

:shipit:

Member

jujugui commented Mar 16, 2016

Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/bundlechanges-merge

jujugui added a commit that referenced this pull request Mar 16, 2016

@jujugui jujugui merged commit 383c581 into juju:master Mar 16, 2016

1 check passed

default Merged build finished.
Details

@ericsnowcurrently ericsnowcurrently deleted the ericsnowcurrently:resources-add-to-service branch Mar 16, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment