Skip to content

minor fixes and optimizations#608

Closed
brainfoolong wants to merge 4 commits intoleafo:masterfrom
brainfoolong:master
Closed

minor fixes and optimizations#608
brainfoolong wants to merge 4 commits intoleafo:masterfrom
brainfoolong:master

Conversation

@brainfoolong
Copy link
Contributor

Hi, just give you a little push in code readability and optimization. Also fixed a php error because of undefined variable.

@Cerdic
Copy link
Contributor

Cerdic commented Apr 23, 2019

Checked this PR today, it's safe and ready to be pulled

@mahagr
Copy link

mahagr commented Apr 24, 2019

Do you have a branch with all your changes applied?

@Cerdic
Copy link
Contributor

Cerdic commented Apr 24, 2019

Yes i'm putting everything all together here https://github.com/Cerdic/scssphp/tree/maint/leafo/master but this is a work in progress, I'm still working on the #627 integration wich is at the moment coming with bugs

@robocoder
Copy link
Collaborator

I cherry-picked the commits from this PR with the exception of 4cc12d6. Currently, we use explicit use statements. In practice, I find this makes some refactoring easier (e.g., when changing the namespace of a class).

@robocoder robocoder closed this Apr 24, 2019
switch ($value[0]) {
case Type::T_EXPRESSION:
if ($value[1] === '/') {
return $this->shouldEval($value[2], $value[3]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This isn't just a code smell. It looks like a bug.

robocoder added a commit that referenced this pull request Apr 24, 2019
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.

4 participants