Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade ember-code-snippet to v3.0.0 #436

Merged

Conversation

josemarluedke
Copy link
Contributor

@josemarluedke josemarluedke commented Dec 13, 2019

This was a major version bump with some breaking changes: https://github.com/ef4/ember-code-snippet/blob/master/CHANGELOG.md#300

The notable change here is that they don't supply syntax highlight component out of the box anymore. We had highlight.js already, so it was just a matter of wiring everything together.

Note on the minimal style changes, I was looking at MirageJS docs site, and copied some changes from them as IMO looks a bit better.

cc/ @samselikoff @rwwagner90

@RobbieTheWagner
Copy link
Member

@josemarluedke does this change the user facing API at all?

@josemarluedke
Copy link
Contributor Author

Nope, no changes for addon-docs consumers.

Copy link
Member

@RobbieTheWagner RobbieTheWagner left a comment

Choose a reason for hiding this comment

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

Nope, no changes for addon-docs consumers.

With that in mind, this looks good to me!

@samselikoff any concerns?

@samselikoff
Copy link
Contributor

Wow, thanks so much for doing this @josemarluedke ! Been meaning to work on this forever.

Looks like there are some failures in 2.12/2.18 🤔

@josemarluedke
Copy link
Contributor Author

@samselikoff tests are passing now. The let helper is not available in ember v2.x.

@samselikoff samselikoff merged commit 73f9eb1 into ember-learn:master Dec 13, 2019
@samselikoff
Copy link
Contributor

Thanks man, nice work!

@josemarluedke josemarluedke deleted the jl-upgrade-ember-code-snippet branch December 13, 2019 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants