Skip to content

Conversation

@dcousens
Copy link
Contributor

It is not our responsibility to uglify the code.

If they want to uglify it, there are plenty of tools to which they could use, just like the rest of their build process.

@weilu thoughts?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.25% when pulling fa96764 on nouglify into 7ad3ac5 on master.

dcousens added a commit that referenced this pull request Feb 13, 2015
@dcousens dcousens merged commit 437b25e into master Feb 13, 2015
@dcousens dcousens deleted the nouglify branch February 13, 2015 01:02
@dcousens
Copy link
Contributor Author

@weilu still happy to hear feedback, but I can't foresee any real issue with this.

@weilu
Copy link
Contributor

weilu commented Feb 16, 2015

Here's a reason why this change is a bad idea:

±  |master ✗| → ll -h bitcoin.js 
-rw-r--r--@ 1 wlu  401060983   620K Feb 16 19:47 bitcoin.js

±  |master ✗| → ll -h bitcoinjs-min.js 
-rw-r--r--@ 1 wlu  401060983   149K Oct  7 15:34 bitcoinjs-min.js

@weilu
Copy link
Contributor

weilu commented Feb 16, 2015

After I posted it I realized that the minified bitcoinjs was of a much older version. Current master yields:

±  |master ✗| → ll -h bitcoinjs-min.js 
-rw-r--r--@ 1 wlu  401060983   424K Feb 16 19:52 bitcoinjs-min.js

@dcousens I'm curious to what makes bitcoinjs-lib 3x more bloated than 4 months ago

@dcousens
Copy link
Contributor Author

Crypto-browserify has grown substantially in size. That may accommodate
for some of it.
On 16 Feb 2015 8:02 pm, "Wei Lu" notifications@github.com wrote:

After I posted it I realized that the minified bitcoinjs was of a much
older version. Current master yields:

± |master ✗| → ll -h bitcoinjs-min.js
-rw-r--r--@ 1 wlu 401060983 424K Feb 16 19:52 bitcoinjs-min.js

@dcousens https://github.com/dcousens I'm curious to what makes
bitcoinjs-lib 3x more bloated than 4 months ago


Reply to this email directly or view it on GitHub
#358 (comment)
.

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.

4 participants