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

Switch from grunt to gulp for more flexible build #18

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

pavelhoral
Copy link
Member

This is mainly about personal preference, but there are also few minor advantages to changing from Grunt to Gulp:

  • from the two task runners, Gulp is definitely the more modern one (even though Gulp can feel a bit rusty compared to other hype tools, we can't get completely rid of generic task runner)
  • Gulp supports modern day JS much much better (mainly talking about ESM and async API)
  • it is far easier to write clean and compact custom tasks with Gulp than with Grunt (e.g. when task configuration has to be dynamically constructed)
  • I am not a huge fan of Grunt's codebase (large single-procedure scripts)
  • I would like to move Maven's responsibilities to JS world (i.e. get rid of maven-resources-plugin)
  • task runners are not playing that important role in the project and are not shared so we can switch without any major impact

@pavelhoral pavelhoral force-pushed the feat-update2 branch 2 times, most recently from 3155920 to b9d1720 Compare November 13, 2023 15:11
@pavelhoral pavelhoral changed the title Switch from grunt to gulp for cleaner build config Switch from grunt to gulp for more flexible build Nov 13, 2023
@karelmaxa karelmaxa self-requested a review November 14, 2023 08:29
return new Promise((resolve, reject) => {
requirejs.optimize({
baseUrl: TARGET_PATH,
mainConfigFile: join(SOURCE_PATH, "js//main.js"),
Copy link
Member

Choose a reason for hiding this comment

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

Double slash is intentional?

Copy link
Member

@karelmaxa karelmaxa left a comment

Choose a reason for hiding this comment

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

LGTM.

@karelmaxa karelmaxa merged commit 33a77b4 into WrenSecurity:main Nov 14, 2023
2 checks passed
@pavelhoral pavelhoral deleted the feat-update2 branch November 14, 2023 12:36
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