-
Notifications
You must be signed in to change notification settings - Fork 331
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
Refactor file copying tasks to remove Gulp #3456
Conversation
a47ebdb
to
3891d17
Compare
7887cab
to
69b3f08
Compare
05724cf
to
e1697b0
Compare
e1697b0
to
344c7c6
Compare
344c7c6
to
ca25c50
Compare
ca25c50
to
72de3bf
Compare
72de3bf
to
1acef41
Compare
1acef41
to
d527c66
Compare
d527c66
to
1dc79d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
The only diff in the outputted JSON files is that they don't have EOLs now. The version file in dist is exactly the same, and I like that we've dropped a few dependencies.
Nice stuff.
Did some unscientific bulk testing of npm run build:package
and there's nothing in it for speed.
I don't think the resulting code is much lengthier or less understandable compared to the gulp-based approach.
Thanks @domoscargin Can you double check your diff direction? After this PR merges we should see trailing newline characters (as a consistency improvement) in Although to reduce the diff size I've kept the 4-space indent (rather than our 2-space default) |
🤦 I doublechecked this before commenting and still somehow got it the wrong way round! |
@domoscargin Don't worry, it caught me out when I wrote it! Cheers |
This PR removes Gulp from the
generateFixtures()
generateMacroOptions()
tasks to complete: