thoughtbot / bitters

Add a dash of pre-defined style to your Bourbon.
https://thoughtbot.com
MIT License
1.39k stars 142 forks source link

Use !default in _variables.scss #195

Closed maryisdead closed 9 years ago

maryisdead commented 9 years ago

I can't see any disadvantage in flagging all variables with !default. This allows to override them in an external file and makes updating Bitters a breeze (well, except for that @import "grid-settings"; statement, but this is less of a pain).

It's already done like that for Neat settings.

tysongach commented 9 years ago

Hi @maryisdead! Bitters is not a library like Bourbon and Neat are. Instead, it’s more of a scaffold, a sort of “jumping off point” to start new projects. So we expect Bitters to be installed towards the beginning of a project, directly edited and not updated like a library would be. This is also why we don’t publish to Bower, npm, etc.

For that reason, the !default flags are unnecessary and would lead to confusion.

maryisdead commented 9 years ago

Hey, thanks for the heads-up and the link, which led to many, many more issues with the same request.

I now know your stance on this particular subject and surely you must be fed up at least a little bit with people asking for it again and again. But here goes … :-)

I just don't get why you seem to be so stubborn about adding !default. It wouldn't change a thing for Bitters, it would still be a scaffold rather than a library, if you like it to be that, and you still wouldn't have to care about backwards-compatibility, because, well, it's a scaffold in the end. ;-) People using it as a drop-in library for sure can take care of that by themselves.

And those same people will still use it as a library, replace their version with a new one, add !default again where needed and go on. I just don't really see a need for that hassle if the changes required don't affect your intentions for Bitters in any way.

Anyways, no need to reply on my rant. I'm still very happy with all the stuff you provide us with and rather live with that minor nuisance than not having Bitters at all. Cheers!

tysongach commented 9 years ago

@maryisdead No worries! Happy to chat about this.

You’re totally right in that the functionality of Bitters would effectively not change if we added !default flags. But I really feel that if we did that, then the code would seem to indicate that this should be treated like a library, rather than a starting point and something you should make your own, on a per-project basis.

Truthfully, I'm struggling to see why someone would want to have variables defined twice (one overriding the other, if we did have !default flags in place) and would love a real-world example for when it would be beneficial. Do you have one, by chance?

Is it because we want to install Bitters in an already-established project and there might already be—for example—a $base-font-size variable defined? If that's the case, then I would argue the code base needs a bit of pruning and whichever $base-font-size is being overridden should be deleted. It goes back to having a Single Source of Truth, which !default complicates, in this case.

On a less serious note: having a bunch of !default flags throughout the _variables partial—to me—is just adding unnecessary code and out of the thousands of project's we've built using Bitters, it's never been a desire to have them. It would just look un-organized.

Cheers!

maryisdead commented 9 years ago

Nah, unfortunately no real-world example that would really demonstrate the dire need to overwrite those variables in an external file.

I guess, just like for most requesting this, it's simply a matter of keeping things clean, especially vendor-provided assets. Best related example is HTML5 Boilerplate (I saw you referring to this in another issue): I tend to drop their styles in and override stuff in another file. If there's updates available it's most likely because of bug fixes or because betters ways have emerged to handle certain things. Now If I had changed everything in their main.css to fit my needs, merging their updates in would simply mean more work.

I know I have done this with Bitters at least three times whilst working on my latest project, just to keep things up-to-date until final release. Surely, this won't happen so often (if at all) after release, but for the time being, !defaultwould've been a time-saver.

tysongach commented 9 years ago

@maryisdead Gotcha. I think we fundamentally see Bitters in a slightly different way. I don’t consider Bitters a vendor asset. Sure, we created it, but once it’s in your project, I look at it as though it’s now part of your project’s core styles. This is why we install it as a directory called base/ (as opposed to bitters/).

Bitters is also not only variables. So when we talk about “updates” to a Bitters installation, it will almost always involve some manual effort so to not override any custom base element styles you may have changed. For example, maybe you customized some styles in _typography, like changing heading margins and putting the text-decoration on a tags (things that might not necessarily be tied to a variable). A simple “update” to the latest version of Bitters would overwrite those, if you’re not careful. So the only way would be to manually pull in any new stuff from the latest version of Bitters.

Actually, now that I think more about it, the variables are probably one of the least changed things when we update. And when we do update (which is not super often; only two in all of 2015 so far) they are usually small refinements, not major swooping changes. So reviewing the changelog—which we are getting better about documenting—and manually carrying over any changes you need for your project, seems completely doable and not very time-consuming. A more time-consuming and fragile setup—in my opinion—is closely coupling to and overriding Bitters styles and leads to not having a single source of truth.

Finally, for the record, there’s an old PR for adding !default flags which I forgot about and would also provide context: https://github.com/thoughtbot/bitters/pull/32