Skip to content

Conversation

@jouve
Copy link
Contributor

@jouve jouve commented Jan 17, 2013

error is that enabled ||= true always evaluates to true.
Change all initialization of bool settings to use the same syntax:
setting = true if setting.nil?

Same as #2652 but for master.

error is that ```enabled ||= true``` always evaluates to true.
Change all initialization of bool settings to use the same syntax:
```setting = true if setting.nil?```
riyad added a commit that referenced this pull request Jan 18, 2013
Fix default settings when they are boolean.
@riyad riyad merged commit b07e1b3 into gitlabhq:master Jan 18, 2013
@jouve jouve deleted the fix_default_settings branch January 18, 2013 03:11
@riyad
Copy link
Contributor

riyad commented Jan 18, 2013

Ouch ... I feel really stupid. 😞 I had to read it a few times until I got it ... I feel sorry for the person who proposed this fix a few days ago and it got turned down ... if you are reading this: Sorry. 😅

For the ones who still don't get it: ;)
We have boolean options which we need to set to a default value if they are not set. So if the option is nil we assume it is not set and set a default value. If it is false we assume it was set deliberately, so we leave it.
Can anyone see it now? ... I'll tell you:
We use the ||= operator which will assign the value on the right if the left-hand expression evaluates to false. In our case we would set the default value regardless whether the option was nil or false thus overwriting a user setting. 😱 (btw that's the reason we were getting those "I disabled Gravatar, but it isn't really disabled" issues 😅)

So now we explicitly check and only overwrite boolean options if they are nil. Thanks again @jouve. 👍

@riyad riyad mentioned this pull request Jan 18, 2013
@riyad
Copy link
Contributor

riyad commented Jan 18, 2013

@jouve I see it was you in #2537 who proposed this fix earlier ... sorry 😅

@kaustavdm
Copy link

👍 😄

stanhu added a commit that referenced this pull request Sep 23, 2015
rspeicher pushed a commit that referenced this pull request Sep 23, 2015
Make commit graphs responsive to window width changes

Closes #2653

See merge request !1395
stanhu added a commit that referenced this pull request Sep 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants