-
Notifications
You must be signed in to change notification settings - Fork 163
Remove unnecessary joinClasses dependency #57
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
Remove unnecessary joinClasses dependency #57
Conversation
This is fine in principle, except lib/ contains the transpiled sources. You'll want to edit the file in src/.
d2476fe to
4bb05c5
Compare
😁 sorry, fixed.
Sorry, one last thing - don't update the files in lib/. We'll handle it when we release.
@mtscout6 What do you think of .gitignoreing the lib/ path and putting npm run build in the prepublish script?
LGTM otherwise, need another @react-bootstrap/collaborators reviewer though.
What do you think of .gitignoreing the lib/ path and putting npm run build in the prepublish script?
+1 👍 It's making this one-line review a pain, for instance.
LGTM.
What do you think of .gitignoreing the lib/ path and putting npm run build in the prepublish script?
👍
I would prefer to have the lib changes stripped before we pull this in, but that's just me. Looks good otherwise.
@aabenoja I agree, I'll make a PR once we merge this.
I agree, I'll make a PR once we merge this.
Isn't that contradictory? 😛
4bb05c5 to
cfeff68
Compare
Removed lib changes, guys, it's pretty clean now.
Remove unnecessary joinClasses dependency
@taion Once we setup a separate repo for the bower release and put scripts in place to automate that then I'd be fine with removing the lib folder from the repo. The only reason it's committed now is for the bower release.
See issue #48
Brain fart, forgot that Bower doesn't work like NPM and actually looks at the repo.
...which also ruins builds with React from CDN.