Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Exclude more unneeded files in dist builds #10219

Merged
merged 1 commit into from
Mar 18, 2015

Conversation

marcelgerber
Copy link
Contributor

This Gruntfile excludes way more unnecessary files (mostly test/, doc/ and examples/), which results in a dist folder taking up way less space than before (resulting in a smaller installer).

Disk space usage analysis (using RidNacs):
On master:
image

On dist-exclude:
image

That's 10MB less!
I haven't tested this at all, though.

@marcelgerber marcelgerber force-pushed the dist-exclude branch 2 times, most recently from c43f6cf to bcc4e07 Compare February 28, 2015 12:55
@nethip
Copy link
Contributor

nethip commented Mar 11, 2015

@marcelgerber Good to see this..

@nethip nethip self-assigned this Mar 11, 2015
@marcelgerber
Copy link
Contributor Author

@nethip Just so you know, while most changes should be safe, in bcc4e07 I included another step to uglify all CodeMirror files. I haven't tested it all that much (things to test are using multiple modes, CM addon loading, maybe even keymap loading, CM theme loading).
If something unexpected happens, it's always easy to revert that tiny little change.

@nethip
Copy link
Contributor

nethip commented Mar 12, 2015

@marcelgerber I think this is still a good change except for my doubts on having to uglify Code Mirror JS library. Can we drop off the uglify build step and proceed with the rest. I will try to unit test this today.

@marcelgerber
Copy link
Contributor Author

Do you want me to still uglify the nls (translation) files or should I drop that step as well?

@nethip
Copy link
Contributor

nethip commented Mar 12, 2015

Yeah nls as well. You could put up a different PR for the uglify step alone, which probably might need rigorous testing.

@marcelgerber
Copy link
Contributor Author

@nethip I've removed the extra build step.

Also, I've taken new screenshots of the savings we achieve:
On this branch (dist-exclude):
image

On master branch (master):
image

@nethip
Copy link
Contributor

nethip commented Mar 13, 2015

@marcelgerber I pulled down your change and ran grunt build. I could see that test, example, docs folders, present right under extensibility/node/node_modules are getting squashed. However, If the node_modules contain further node_modules, the nested ones are not being considered for the exclusion. Is there a way we can fix this? If this is fixed, we can further bring down the foot print.

@nethip
Copy link
Contributor

nethip commented Mar 13, 2015

And also I built the MAC installer and I could see the size reduce by ~7MB. 😄

@marcelgerber
Copy link
Contributor Author

Hm, IIRC I didn't see this when testing (can't test right now).
Is it possible those folders are empty?

@nethip
Copy link
Contributor

nethip commented Mar 13, 2015

I saw that the folders were not empty.

What I meant above was that the exclusion rules get applied properly but only for extensibility/node/node_modules/*.The rule would not exclude for e.g. the following folder.

extensibility/node/node_modules/fs-extra/node_modules/mkdirp/examples

Can we have a recursive rule which would plainly eliminate all the example/test/docs folders, given the path to the root folder?

@marcelgerber
Copy link
Contributor Author

For me, the folder is actually empty:
image

That's also what I'd expect from the wildcard !extensibility/node/node_modules/**/examples/**/* (notice the node_modules/**/ part)

@nethip
Copy link
Contributor

nethip commented Mar 13, 2015

Hmm. That's interesting. Let me check and get back to you on this.

@nethip
Copy link
Contributor

nethip commented Mar 16, 2015

@marcelgerber Yes! You are right. I see the files getting deleted but empty folders still being present. Is there a way we can get rid of the empty folders too?

@nethip
Copy link
Contributor

nethip commented Mar 17, 2015

@marcelgerber I was not able to find documentation on how to tell grunt-contrib-copy to delete empty folders which became empty because of the exclusions. So have added one more task to Grunt(grunt-cleanempty)- . Checkout this commit. 1889c94. This would clean all the empty directories that got created in the copy phase because of file exclusions. After running the task this is the result

Running "cleanempty:src" (cleanempty) task
>> 1553 paths cleaned.

CC @ingorichter : Ingo do these changes sound good to you?

@marcelgerber
Copy link
Contributor Author

@nethip I merged the commit you mentioned (thanks!).

Please let me know when you'd like to merge so I can rebase.

@nethip
Copy link
Contributor

nethip commented Mar 17, 2015

Thanks @marcelgerber I would like to hear from ingo. @ingorichter could you have a look at this.

@nethip
Copy link
Contributor

nethip commented Mar 18, 2015

@marcelgerber Go ahead and push your changes to your fork. I will go ahead with the merge.

@marcelgerber
Copy link
Contributor Author

Ha, you commented the perfect time so I could quickly rebase it ;)

Just rebased, everything ready for a merge,

@nethip
Copy link
Contributor

nethip commented Mar 18, 2015

Thanks @marcelgerber . Do you want to merge src/config.json as well?

@marcelgerber
Copy link
Contributor Author

Yeah, our grunt task automatically sync it with package.json every time you run grunt.

nethip added a commit that referenced this pull request Mar 18, 2015
Exclude more unneeded files in dist builds
@nethip nethip merged commit 240347f into adobe:master Mar 18, 2015
@nethip
Copy link
Contributor

nethip commented Mar 18, 2015

Looking good. Merged this.. Thanks @marcelgerber !

@marcelgerber marcelgerber deleted the dist-exclude branch March 18, 2015 06:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants