Attempt to Fix all warnings from Sass. #1146

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@vipulnsward

Hi all, This is an attempt to fix different kinds of warnings form

  • Circular dependency loading
  • Un- initialized variable.
  • Use of File.exists? instead of File.exist?
  • Parenthesis around ambiguous arguments, etc.

Not sure what the policy is about this. Just an attempt to clear out some warnings I see on Sinatra builds. (https://travis-ci.org/sinatra/sinatra/jobs/19503469)

Fix warnings from-
* Circular dependency loading
* Un- initialized variable.
* Use of File.exists? instead of File.exist?
* Paranthesis around ambiguous arguments, etc.
@nex3

This comment has been minimized.

Show comment Hide comment
@nex3

nex3 Mar 7, 2014

Contributor

We're not interested in avoiding warnings caused by uninitialized instance variables. We consider instance variables defaulting to nil to be a feature of Ruby, and use it accordingly.

If you remove the instance-variable changes, I'll merge this.

Contributor

nex3 commented Mar 7, 2014

We're not interested in avoiding warnings caused by uninitialized instance variables. We consider instance variables defaulting to nil to be a feature of Ruby, and use it accordingly.

If you remove the instance-variable changes, I'll merge this.

@vipulnsward

This comment has been minimized.

Show comment Hide comment
@vipulnsward

vipulnsward Mar 28, 2014

Actually it just returns nil value, doesn't default to setting/initializing it.
ruby/ruby@b996367

Do you wan't to me to remove the initializations to nil, with such a behaviour from Ruby?
Let me know, I'l update the PR.

Actually it just returns nil value, doesn't default to setting/initializing it.
ruby/ruby@b996367

Do you wan't to me to remove the initializations to nil, with such a behaviour from Ruby?
Let me know, I'l update the PR.

@nex3

This comment has been minimized.

Show comment Hide comment
@nex3

nex3 Apr 4, 2014

Contributor

Yes, please do remove the nil initializations.

Contributor

nex3 commented Apr 4, 2014

Yes, please do remove the nil initializations.

@vipulnsward

This comment has been minimized.

Show comment Hide comment
@vipulnsward

vipulnsward Aug 9, 2014

I see that all changes are already in master.

I see that all changes are already in master.

@vipulnsward vipulnsward closed this Aug 9, 2014

@sds sds referenced this pull request in brigade/scss-lint Sep 4, 2014

Closed

Sass 3.4 #218

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment