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

Remove require-dir dependency #981

Merged
merged 1 commit into from
Jan 23, 2019
Merged

Remove require-dir dependency #981

merged 1 commit into from
Jan 23, 2019

Conversation

wincent
Copy link
Contributor

@wincent wincent commented Jan 23, 2019

This dependency doesn't do much for us: removing it shows that we can get by without it (note how we used four lines invoking the package before this commit, and when we remove the package we can get an equivalent outcome with four lines, leaving the total count unchanged).

In any case, the real motivation here is that when we move to Gulp v4 (see #980), the pattern is to refer to tasks using function references rather than strings, which means we'd have to manually import the "build" tasks additionally anyway. Reducing our dependency footprint is just a bonus.

Test plan: npm run build and see that everything still works.

This dependency doesn't do much for us: removing it shows that we can
get by without it (note how we used four lines invoking the package
before this commit, and when we remove the package we can get an
equivalent outcome with four lines, leaving the total count unchanged).

In any case, the real motivation here is that when we move to Gulp v4
(see #980), the pattern is to refer to tasks using function references
rather than strings, which means we'd have to manually import the
"build" tasks additionally anyway. Reducing our dependency footprint is
just a bonus.

Test plan: `npm run build` and see that everything still works.
@wincent wincent requested a review from julien January 23, 2019 13:40
@wincent wincent merged commit e41a1e0 into liferay:2.x-develop Jan 23, 2019
@wincent wincent deleted the require-dir branch January 23, 2019 16:26
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