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

gulp: Refactoring #1780

Merged
merged 3 commits into from
Mar 2, 2019
Merged

Conversation

RunDevelopment
Copy link
Member

@RunDevelopment RunDevelopment commented Mar 1, 2019

Since we just switched to gulp v4 (#1779) and dropped support for Node 4 some time ago, I took the chance to refactor our gulpfile.js.

I used ES6 syntax and removed gulp.task which is not recommended anymore.

(Let's see if Node v6 can handle the changes. Edit: It worked.)

Copy link
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

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

LGTM, r+

@RunDevelopment
Copy link
Member Author

@mAAdhaTTah

gulpfile.js Outdated
function build(cb) {
pump([src(paths.main), header('\n/* **********************************************\n' +
' Begin <%= file.relative %>\n' +
'********************************************** */\n\n'), concat('prism.js'), dest('./')], cb);
Copy link
Member

Choose a reason for hiding this comment

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

If we're ES6-ifying everything, we can make this a template string.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'********************************************** */\n\n'), concat('prism.js'), dest('./')], cb);
********************************************** */
`), concat('prism.js'), dest('./')], cb);

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't do this because then the line ends of prism.js depend on the line ends of gulpfile.js.

But if you think that that's not a problem I'll change it.

Copy link
Member Author

Choose a reason for hiding this comment

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

nvm. Template strings ftw! I already use them anyway for components.js.

Copy link
Member

@mAAdhaTTah mAAdhaTTah left a comment

Choose a reason for hiding this comment

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

one nit then gtg

Copy link
Contributor

@ExE-Boss ExE-Boss left a comment

Choose a reason for hiding this comment

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

Apply these in a single commit using the files tab:

gulpfile.js Outdated
pump([src(paths.plugins), ...minifyJS(), rename({ suffix: '.min' }), dest('plugins')], cb);
}
function build(cb) {
pump([src(paths.main), header('\n/* **********************************************\n' +
Copy link
Contributor

@ExE-Boss ExE-Boss Mar 2, 2019

Choose a reason for hiding this comment

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

Suggested change
pump([src(paths.main), header('\n/* **********************************************\n' +
pump([src(paths.main), header(`
/* **********************************************

gulpfile.js Outdated
}
function build(cb) {
pump([src(paths.main), header('\n/* **********************************************\n' +
' Begin <%= file.relative %>\n' +
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
' Begin <%= file.relative %>\n' +
Begin <%= file.relative %>

gulpfile.js Outdated
function build(cb) {
pump([src(paths.main), header('\n/* **********************************************\n' +
' Begin <%= file.relative %>\n' +
'********************************************** */\n\n'), concat('prism.js'), dest('./')], cb);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'********************************************** */\n\n'), concat('prism.js'), dest('./')], cb);
********************************************** */
`), concat('prism.js'), dest('./')], cb);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants