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.

Sign up for GitHub

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

Jump to bottom

Compress Option #32

Merged
merged 1 commit into from
Aug 24, 2015
Merged

Compress Option #32

merged 1 commit into from
Aug 24, 2015

Conversation

blond
Copy link
Member

@blond blond commented Aug 24, 2015

Resolved #30

@blond blond added the review label Aug 24, 2015
tormozz48
tormozz48 reviewed Aug 24, 2015
View reviewed changes
techs/browser-js.js
@@ -19,6 +20,7 @@ var EOL = require('os').EOL;
* involved in the assembly.
* @param {Boolean} [options.iife=false] Adds an option to wrap merged<br>
* files to IIFE.
* @param {Boolean} [options.compress=false] Minifies and compress JS code.
Copy link
Contributor

Choose a reason for hiding this comment

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

Мы же обычно пишем что-то вроде: 'Set true to ...'

Copy link
Member

Choose a reason for hiding this comment

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

compress -> compresses

Copy link
Member Author

Choose a reason for hiding this comment

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

Мы же обычно пишем что-то вроде: 'Set true to ...'

Вроде бы нет :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Я не совсем точно написал. Вот например из bemxjst: "Sets devMode option for convenient debugging. If devMode is set to true, code of templates will not be compiled but only wrapped for development purposes."

@tormozz48
Copy link
Contributor

отписал

@tadatuta
Copy link
Member

здесь не возникнет дополнительных проблем с получением сорс-мапов в будущем?

@blond
Copy link
Member Author

blond commented Aug 24, 2015

здесь не возникнет дополнительных проблем с получением сорс-мапов в будущем?

Возникнет, но uglify умеет работать с сорс-мапами.

@blond
Copy link
Member Author

blond commented Aug 24, 2015

🆙

@tormozz48
Copy link
Contributor

🆗

@tadatuta
Copy link
Member

а для ноды сжатие имеет какой-то практический смысл?

@blond
Copy link
Member Author

blond commented Aug 24, 2015

а для ноды сжатие имеет какой-то практический смысл?

Думаю, что нет. Выпилить?

@tadatuta
Copy link
Member

если смысла нет, а при включении станет медленнее, то, кмк, лучше заранее не давать такой возможности

@blond
Copy link
Member Author

blond commented Aug 24, 2015

удалил

tadatuta
tadatuta reviewed Aug 24, 2015
View reviewed changes
techs/browser-js.js
'/* end: ' + relPath + ' *' + '/'
].join(EOL);

// after compress comments will be removed
Copy link
Member

Choose a reason for hiding this comment

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

compress -> compression

@tadatuta
Copy link
Member

lgtm

blond added a commit that referenced this pull request Aug 24, 2015
@blond
Merge pull request #32 from enb-make/issue-30
3a47337 
Compress Option
@blond blond merged commit 3a47337 into master Aug 24, 2015
@blond blond deleted the issue-30 branch August 24, 2015 21:43
@blond blond removed the review label Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@blond @tormozz48 @tadatuta