Skip to content

Commit

Permalink
Add font-awesome.scss and font-awesome.less to bower.json
Browse files Browse the repository at this point in the history 
Fix #6780 #6227
  • Loading branch information
@tagliala @davegandy
tagliala authored and davegandy committed Jul 28, 2015
1 parent bbe0a40 commit 7cde41e
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions bower.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
"devDependencies": {},
"license": ["OFL-1.1", "MIT", "CC-BY-3.0"],
"main": [
"./css/font-awesome.css",
"./fonts/*"

This comment has been minimized.

Sign in to view

Copy link
@rcaught

rcaught Jul 30, 2015

This has broken my sites due to none of the .ttf files being exposed now.

This comment has been minimized.

Sign in to view

Copy link
@fridjon

fridjon Aug 5, 2015

+1 on breaking.

This comment has been minimized.

Sign in to view

Copy link
@tagliala

tagliala Aug 5, 2015

Author Member

sorry, please use overrides

This comment has been minimized.

Sign in to view

Copy link
@rcaught

rcaught Aug 5, 2015

Just to note: This commit does break the semantic versioning guidelines.

This comment has been minimized.

Sign in to view

Copy link
@fridjon

fridjon Aug 5, 2015

Thanks, overriding works. In case someone is not familiar with the override mechanism here is what I needed to do in my bower.json:

  "overrides": {
    "font-awesome": {
      "main": [
        "./css/font-awesome.css",
        "./fonts/*"
      ]
    }
  }

This comment has been minimized.

Sign in to view

Copy link
@cmckni3

cmckni3 Jun 8, 2016 •
edited
Loading

I had to add an override as well.

"font-awesome": {
  "main": [
    "./fonts/fontawesome-webfont.ttf",
    "./fonts/fontawesome-webfont.woff",
    "./fonts/fontawesome-webfont.woff2"
  ]
}
"less/font-awesome.less",
"scss/font-awesome.scss"
],
"ignore": [
"*/.*",
Expand Down

15 comments on commit 7cde41e

@zhuangya
Copy link

Choose a reason for hiding this comment

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

hmm, why remove font-awesome.css by default?

@zhuangya
Copy link

Choose a reason for hiding this comment

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

it could be a problem if using something like wiredep :(
i have to override font-awesome's bower main in my bower.json

@fracz
Copy link

@fracz fracz commented on 7cde41e Jul 29, 2015

Choose a reason for hiding this comment

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

Exactly, now we have to add special config in in overrides section of bower.json to have FontAwesome included properly...

"overrides": {
  "font-awesome": {
    "main": [
      "./css/font-awesome.css",
      "./fonts/*"
    ]
  }
}

But, previously it worked OOTB...

@zhuangya
Copy link

Choose a reason for hiding this comment

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

i just checked #6780 and #6227
all due to wiredep workflow..

it could be a problem if write both less/scss and css in main, when work with less/scss and wiredep workflow..

hmm...

@tagliala
Copy link
Member Author

Choose a reason for hiding this comment

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

I followed bower's specs, please use overrides

@notbrain
Copy link

Choose a reason for hiding this comment

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

This was a nice surprise. Totally broke our build system. The bower.json file should NOT assume that less or scss compilation need to take place after copying files (with bower-main for example). It should be a packaged library (which means point to baked css and the font files)!

If people want to include the scss or less files and customize them with their build, then using overrides is appropriate, not the other way around!

UPDATE: ok so after the panic of "why isn't FA being included in my build anymore?" wore off, it's now clear that the de-facto meaning of main has now been betrayed, and we're getting a new files array in bower.json to denote which files to pick up if we just want to run straight in the browser? Doesn't really make sense to me to break a bunch of systems like this. Especially now that we'll have to either patch stuff ourselves or wait for other tools to be updated. Sigh.

@zhuangya
Copy link

Choose a reason for hiding this comment

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

ref twbs/bootstrap@55336e6

@cjroth
Copy link

@cjroth cjroth commented on 7cde41e Aug 7, 2015

Choose a reason for hiding this comment

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

This was frustrating for me too, so I asked:

bower/bower.github.io@5fb9b63#commitcomment-12590301

@ThisGuyCodes
Copy link

Choose a reason for hiding this comment

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

look, I get that there's a spec, and people want to follow it. But if a change is going to break a lot of people's workflows (which were established before there was a spec), you need to consider it a breaking change, and do a major version increment. Again, yes, it's to conform with the spec, which is all well and good, but since there wasn't one before, you can't blindly expect all existing users to be following it. Sorry, but the tons of people and companies can't do a massive workflow overhaul overnight once a specification is published.

Do a major version increment.

@zhuangya
Copy link

Choose a reason for hiding this comment

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

look, I get that there's a spec, and people want to follow it. But if a change is going to break a lot of people's workflows (which were established before there was a spec), you need to consider it a breaking change, and do a major version increment. Again, yes, it's to conform with the spec, which is all well and good, but since there wasn't one before, you can't blindly expect all existing users to be following it. Sorry, but the tons of people and companies can't do a massive workflow overhaul overnight once a specification is published.

Do a major version increment.

totally agree!

@tagliala
Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, this was an unintended breaking change

Please use override

@ThisGuyCodes
Copy link

Choose a reason for hiding this comment

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

@tagliala http://semver.org/ states (and even if you're not adhering strictly to semver, this is still sound advice):

What do I do if I accidentally release a backwards incompatible change as a minor version?

As soon as you realize that you've broken the Semantic Versioning spec, fix the problem and release a new minor version that corrects the problem and restores backwards compatibility. Even under this circumstance, it is unacceptable to modify versioned releases. If it's appropriate, document the offending version and inform your users of the problem so that they are aware of the offending version.

@tagliala
Copy link
Member Author

Choose a reason for hiding this comment

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

@conslo I can't take this decision, it is up to @davegandy

If it is ok for him, we can revert the offending commit and schedule the change for 5.0.0

edit: I personally do not want to release another version to fix this. We are following new bower specs, sooner or later you need to use overrides.

Please take a look here: twbs/bootstrap#16663

@xdumaine
Copy link

Choose a reason for hiding this comment

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

@tagliala

I personally do not want to release another version to fix this. We are following new bower specs, sooner or later you need to use overrides.

This is a really selfish choice. If "sooner" means a breaking change in a minor version, then "later" is the answer - a major version.

@tagliala
Copy link
Member Author

Choose a reason for hiding this comment

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

@xdumaine please refer to twbs/bootstrap#16663 twbs/bootstrap#16663 (comment)

Please sign in to comment.