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

Use npm for Basscss CSS framework #1418

Merged
merged 1 commit into from
May 8, 2017
Merged

Use npm for Basscss CSS framework #1418

merged 1 commit into from
May 8, 2017

Conversation

hursey013
Copy link
Member

The version of Basscss in the vendor/ folder was a custom combination of v3.0.0 and v4.0.0. This PR removes the vendor/ dependency and uses npm to import v3.0.0. All that remains in vendor/ are the necessary stylesheets from v4.0.0.

@hursey013 hursey013 self-assigned this May 4, 2017
@hursey013 hursey013 requested a review from el-mapache May 4, 2017 18:36
@el-mapache
Copy link
Contributor

I'm sure this is a super obvious question, but can we just use v 4.0 and remove 3 entirely?

@hursey013
Copy link
Member Author

@el-mapache yeah, that's what I would have preferred, but the 4.0.0 version has a bunch of breaking changes, and they are things that we are currently relying on: https://github.com/basscss/basscss/releases/tag/8.0.0

It's definitely possible, it just wouldn't be a drop in solution.

@el-mapache
Copy link
Contributor

that is indeed a lot of breaking changes

Copy link
Contributor

@el-mapache el-mapache left a comment

Choose a reason for hiding this comment

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

looks good, lets merge

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

Looks like CSS to me, if the site still renders correctly, then LGTM! 👍

@hursey013 hursey013 merged commit fb96867 into master May 8, 2017
@hursey013 hursey013 deleted the bh-basscss-npm branch May 8, 2017 21:35
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