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 actual colors in SASS variables. Rework of #1542. #4167

Merged
merged 1 commit into from
Mar 3, 2016

Conversation

sgomes
Copy link
Contributor

@sgomes sgomes commented Mar 2, 2016

This PR replaces our color variables to use actual colors rather
than strings. It also updates the template generation and
customizer to use '#name' style placeholders instead of '$name',
so that CSSO doesn't complain about the syntax.

This PR may later be invalidated if we decide to move away from SASS,
but for now it provides a good baseline for current development.

@Garbee, @surma: PTAL!

This PR replaces our color variables to use actual colors rather
than strings. It also updates the template generation and
customizer to use '#name' style placeholders instead of '$name',
so that CSSO doesn't complain about the syntax.

This PR may later be invalidated if we decide to move away from SASS,
but for now it provides a good baseline for current development.
@Garbee
Copy link
Collaborator

Garbee commented Mar 2, 2016

LGTM.

Looks like I can dump my local branch with the same import since this is doing the same thing correct? (I'm still like halfway through conflicts so, making sure before I dump the work.)

@sgomes
Copy link
Contributor Author

sgomes commented Mar 2, 2016

Oops, sorry @Garbee, didn't know you already had a branch doing the same thing! Yes, this does the same thing as #1542, with the added support for gulp-csso 1.1, which doesn't like $variables any more.

@Garbee
Copy link
Collaborator

Garbee commented Mar 2, 2016

Don't worry about getting it done. I started the merge after our meeting the other week. But, there are a lot of conflicts so I was just slowly doing it as I had time. I was just doing the slow/hard way to try and keep the commit history.

Thanks for getting it done though, it is much needed.

@Garbee
Copy link
Collaborator

Garbee commented Mar 2, 2016

Restarted Drone on this one. Looks like a timeout occurred for some reason, didn't look like an actual failure of the build.

@surma
Copy link
Contributor

surma commented Mar 3, 2016

Code LGTM. Drone is timing out. Restarted it. Let’s see.

@sgomes
Copy link
Contributor Author

sgomes commented Mar 3, 2016

FYI: neither of you managed to restart Drone, even though I'm sure the UI seemed to indicate you did.

You're likely not logged in, like I wasn't, so even if the button is shown and you click it and it seems like something is happening, nothing actually is. You'll need to go to https://ci.cloudware.io/ and re-login. Sigh.

sgomes added a commit that referenced this pull request Mar 3, 2016
Use actual colors in SASS variables. Rework of #1542.
@sgomes sgomes merged commit 90765ce into master Mar 3, 2016
@sgomes sgomes deleted the color-variable-rework branch March 3, 2016 13:27
@sgomes sgomes mentioned this pull request Mar 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants