Skip to content

Conversation

@alexlafroscia
Copy link
Contributor

  • Used in two places, on of which was broken. Split out into a separate function that both locations can import
  • Only the docs-header used the addonLogo property, all other references to the same feature use logo. The documentation also used logo but the implementation was setting addonLogo and ignoring the provided logo property. Now it's all just logo

logo = 'ember-cli';
} else if (name.match('ember-data-')) {
logo = 'ember-data';
} else if (name.match('ember-data-')) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was using ember-data- twice which didn't make sense


return logo;
}),
addonLogo: addonLogo(packageJson),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unsure if you prefer passing packageJson into the CP macro or would rather this.get('packageJson') within the CP definition. I like this API better since it makes less assumptions about that packgeJson property being available.

This also updates the implementation to match the documentation
@samselikoff
Copy link
Contributor

This is great - thank you dude!

@samselikoff samselikoff merged commit 116f79a into ember-learn:master Mar 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants