Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix and update Bower config #11350

Merged
merged 3 commits into from Jun 26, 2018

Conversation

ncoden
Copy link
Contributor

@ncoden ncoden commented Jun 25, 2018

Description

Update the Bower config file bower.json and fix various issues related to outdated/incorrect configuration.

Changes:

  • Update Bower ignored files list
  • Add description, homepage, keywords, moduleType and repository
  • Add back dependencies (see below)

Note: This PR is suitable for v6.5.

Motivation and Context

Bower dependencies in bower.json were removed in #11073 in the migration from Bower to npm.

These dependencies should not have been removed as the Bower package still require jquery and what-input. Someone installing foundation-sites with bower should still have these packages included. We simply don't use Bower anymore in development mode and don't promote its usage in the documentation.

Removing bower install commands was sufficiant for this migration and Bower dependencies in bower.json were unrelated.

Types of changes

  • Documentation
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing
    functionality to change)

Checklist (all required):

  • I have read and follow the CONTRIBUTING document.
  • There are no other pull request similar to this one.
  • The pull request title is descriptive.
  • The template is fully and correctly filled.
  • The pull request targets the right branch (develop or support/*).
  • My commits are correctly titled and contain all relevant information.
  • My code follows the code style of this project.
  • [ ] I have updated the documentation accordingly to my changes (if relevant).
  • [ ] I have added tests to cover my changes (if relevant).
  • All new and existing tests passed.

@ncoden ncoden added the bug fix label Jun 25, 2018
@ncoden ncoden requested a review from DanielRuf June 25, 2018 23:05
Bower dependencies in `bower.json` were removed in foundation#11073 in the migration from Bower to npm.

These dependencies should not have been removed as the Bower package still require `jquery` and `what-input`. Someone installing `foundation-sites` with bower should still have these packages included. We simply don't use Bower anymore in development mode and don't promote its usage in the documentation.

Removing `bower install` commands was sufficiant for this migration and Bower dependencies in `bower.json` were unrelated.

See foundation#11073
@ncoden ncoden force-pushed the fix/fix-update-bower-config branch from f0c8e2a to e3979a3 Compare June 25, 2018 23:07
@ncoden ncoden added this to the 6.5.0 milestone Jun 25, 2018
@DanielRuf
Copy link
Contributor

Do we still want to support Bower which is dead in general?

@ncoden
Copy link
Contributor Author

ncoden commented Jun 25, 2018

It is dead but people are still using it. So for a project as big as this one I would say: "Keep the support as long as you do not promote its usage and do not spend too much time on it."

@ncoden
Copy link
Contributor Author

ncoden commented Jun 25, 2018

https://twitter.com/kbal11/status/805891962993713152 (but it's 1.5 years old)

@ncoden ncoden merged commit ce589c6 into foundation:develop Jun 26, 2018
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jul 1, 2018
ncoden added a commit to ncoden/foundation-sites that referenced this pull request Jul 6, 2018
…nfig for v6.5.0

54269a7 chore: update Bower ignored files list
5e7143c chore: add various fields to Bower config
e3979a3 fix: add back Bower dependencies

Signed-off-by: Nicolas Coden <nicolas@ncoden.fr>
@ncoden ncoden mentioned this pull request Jul 8, 2018
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants