Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Fix charmstoreURL icon path when no trailing slash #2148

Merged
merged 3 commits into from Oct 20, 2016

Conversation

bac
Copy link

@bac bac commented Oct 20, 2016

No description provided.

@jujugui
Copy link
Contributor

jujugui commented Oct 20, 2016

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

@jujugui
Copy link
Contributor

jujugui commented Oct 20, 2016

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

@@ -2688,6 +2690,8 @@ YUI.add('juju-gui', function(Y) {
*/
charmstore: {},

charmstoreURL: {},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should have a setter with the 'ensureTrailingSlash' method in it

setter(val) {
  return views.utils.ensureTrailingSlash(val);
}

var cfg = window.juju_config,
charmstoreURL = (cfg && cfg.charmstoreURL) || '',
localIndex = charmId.indexOf('local:'),
var charmstoreURL = this.get('charmstoreURL');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work because we aren't in the context of the app instance. you'll either want to do something like this:

// If in app.js
utils.getIconPath.call(this, charmId, isBundle, env);

// If passing to another instance from app.js
utils.getIconPath.bind(this);

// Or you can require charmstoreURL as an argument when calling getIconPath.
utils.getIconPath. bind(this, this.get('charmstoreURL'));

@jujugui
Copy link
Contributor

jujugui commented Oct 20, 2016

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

@bac bac closed this Oct 20, 2016
@bac bac reopened this Oct 20, 2016
@jujugui
Copy link
Contributor

jujugui commented Oct 20, 2016

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

@bac bac force-pushed the handle-charmstoreURL-no-trailing-slash branch from 5fe2549 to 129a795 Compare October 20, 2016 19:36
@jujugui
Copy link
Contributor

jujugui commented Oct 20, 2016

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

Copy link
Contributor

@hatched hatched left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bac !

@hatched hatched added this to the 2.2.3 milestone Oct 20, 2016
@hatched
Copy link
Contributor

hatched commented Oct 20, 2016

:shipit:

@jujugui
Copy link
Contributor

jujugui commented Oct 20, 2016

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

@jujugui jujugui merged commit eb3ad96 into juju:develop Oct 20, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants