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

Move jade to dependencies #26

Merged
merged 2 commits into from
Jun 5, 2016

Conversation

maratfakhreev
Copy link
Contributor

According to #24 and #25 need to move jade from peerDependencies.
The first main reason is that it doesn't install via npm3 and therefore tests fall (have tried after git clone).
The next reason is that in big project application can be installed with wrong jade version and therefore brokes at the start.

"source-map": "~0.5.3",
"browserify-transform-tools": "~1.6.0",
"convert-source-map": "~1.2.0",
"jade": "~1.11.0",
Copy link
Owner

Choose a reason for hiding this comment

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

should we switch to https://www.npmjs.com/package/pug ?

Copy link
Contributor Author

@maratfakhreev maratfakhreev Jun 5, 2016

Choose a reason for hiding this comment

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

I actually tried to use pug and tried to replace all jade requires inside index.js with latest pug release but it doesn't work. Looks like migration on pug a more complex task than simply replace jade on pug. And this is the issue for major release I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sidorares ping

Copy link
Owner

Choose a reason for hiding this comment

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

ok, let's leave jade until resolved

@sidorares
Copy link
Owner

lgtm with one comment

@sidorares sidorares merged commit 55791be into sidorares:master Jun 5, 2016
@maratfakhreev maratfakhreev mentioned this pull request Jun 6, 2016
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