Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Build Tools] update to gulp4 #179

Merged
merged 31 commits into from
Nov 16, 2018
Merged

[Build Tools] update to gulp4 #179

merged 31 commits into from
Nov 16, 2018

Conversation

ColinFrick
Copy link
Member

@ColinFrick ColinFrick commented Oct 17, 2018

Description

Update all dependencies with gulp 4
Admin package was updated, but was not tested

  • Verify css / js / assets build process (full / incremental)
  • Finish "watch" task
  • Test and finish "docs" tasks
  • Batch changes in "watch"
  • Handle deleted files (or skip)

Closes

#112 / Semantic-Org/Semantic-UI#6549 / Semantic-Org/Semantic-UI#3793

@ColinFrick ColinFrick added type/build Anything related to the build process state/on-hold Issues and pull requests which are on hold for any reason labels Oct 17, 2018
@y0hami
Copy link
Member

y0hami commented Oct 17, 2018

Great work @ColinFrick, I will try and get some testing done in the next few days.

@ColinFrick ColinFrick added the state/awaiting-reviews Pull requests which are waiting for reviews label Oct 22, 2018
@ColinFrick ColinFrick added this to the 2.7.x milestone Oct 28, 2018
@ColinFrick ColinFrick self-assigned this Nov 5, 2018
@ColinFrick ColinFrick removed the state/on-hold Issues and pull requests which are on hold for any reason label Nov 7, 2018
prudho
prudho previously approved these changes Nov 7, 2018
Copy link
Contributor

@prudho prudho left a comment

Choose a reason for hiding this comment

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

I have tested most used commands (install, build, watch...) without getting any warning or error. I have checked code and everything looks fine.

Bonus point, Travis built the whole PR all by itself, smoothly.

This is a great job, LGTM 👍

@y0hami
Copy link
Member

y0hami commented Nov 11, 2018

I just did some quick testing and found this error

Site folder exists, merging files (no overwrite) src/site/
[15:51:22] 'create install files' errored after 1.94 ms
[15:51:22] Error: ENOENT: no such file or directory, stat './src/_site'
    at Object.statSync (fs.js:829:3)
    at Object.exports.copyDirSyncRecursive (G:\fomantic\Fomantic-UI\node_modules\wrench-sui\lib\wrench.js:244:23)
    at G:\fomantic\Fomantic-UI\tasks\install.js:341:12
    at taskWrapper (G:\fomantic\Fomantic-UI\node_modules\undertaker\lib\set-task.js:13:15)
    at bound (domain.js:396:14)
    at runBound (domain.js:409:12)
    at asyncRunner (G:\fomantic\Fomantic-UI\node_modules\async-done\index.js:55:18)
    at process._tickCallback (internal/process/next_tick.js:61:11)
[15:51:22] 'install' errored after 2.86 s
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! fomantic-ui@2.6.2 install: `gulp install`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the fomantic-ui@2.6.2 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

Steps:

  1. npm install then went through the installation process selecting all the default values
  2. rm -rf node_modules
  3. npm install this time it asked to extend my config and I selected Yes then Automatic.

@ColinFrick
Copy link
Member Author

ColinFrick commented Nov 11, 2018

Weird because I didn’t really change install

I’ll look into it as soon as I’m home

@ColinFrick
Copy link
Member Author

As I suspected this is a pre-existing problem. Just tested this on master
gulp install removes the _site directory, which other scripts are relying on.

How should we proceed with this problem? This should only affect users who clone the repository, and don't use fomantic-ui throughb npm.

@y0hami
Copy link
Member

y0hami commented Nov 11, 2018

Is there a way to check if _site exists before running the script and then showing a friendly error instead of the script crashing?

@y0hami y0hami modified the milestones: 2.7.x, 2.7.0 Nov 15, 2018
Copy link
Member

@y0hami y0hami left a comment

Choose a reason for hiding this comment

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

👍

Copy link
Member

@lubber-de lubber-de left a comment

Choose a reason for hiding this comment

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

LGTM
Great work! Much appreciated. 👍

@y0hami y0hami merged commit a83a7d4 into fomantic:beta Nov 16, 2018
@ColinFrick ColinFrick deleted the gulp4 branch November 16, 2018 13:41
@y0hami y0hami removed state/awaiting-reviews Pull requests which are waiting for reviews labels Dec 3, 2018
This was referenced Dec 21, 2018
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
type/build Anything related to the build process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants