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

Removed generated files from repository but keep publishing them #18

Merged
merged 5 commits into from
Sep 24, 2015

Conversation

okonet
Copy link
Contributor

@okonet okonet commented Sep 24, 2015

  • removed whitespace from package.json description

"node-libs-browser": "^0.5.2",
"react-tools": "*",
"webpack": "^1.10.0",
"webpack-dev-server": "^1.10.1"
},
"dependencies": {
"peerDependencies": {
"react": "^0.13.3"
Copy link
Owner

Choose a reason for hiding this comment

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

This was causing issues with duplicate Reacts.
I had it before, and removed it intentionally.
If it isn't messing up the new build thing, better remove it?
Or any other ideas? (we at least need a or 0.14-rc thing)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only if you have it in dependencies. I've put into peerDependencies so it will not be installed twice.

Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, I think I had a decent reason of taking it out of peerDependencies :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is strange because in this case you don't define the deps of React at all, which can cause more trouble.

https://nodejs.org/en/blog/npm/peer-dependencies/

also packages like this: YahooArchive/flux-router-component#33 also moved it to peerDependecies

So I think we're safe to go here.

Copy link
Owner

Choose a reason for hiding this comment

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

Accepted it as-is, also added 0.14 as optional dependency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I've noticed it. Thanks! 👍

@borisyankov borisyankov merged commit 3329842 into borisyankov:master Sep 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants