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

chore!: Normalize repository, dropping node <10.13 support #239

Merged
merged 35 commits into from Jan 7, 2024

Conversation

sttk
Copy link
Contributor

@sttk sttk commented Nov 30, 2022

This PR updates this repository for the following points:

  1. Upgrades dependencies and drops node <10.13 support

    • yargs's version is 16, not latest, because the latest version 17 supports node >=12.
    • marked-man version is 0.7, because the latest version 1.3 is esm.
    • eslint, expect, and mocha upgrades to the last versions which supports node v10.
    • Other dependencies upgrades latest versions.
  2. Stops using gulp-test-tools

    Because gulp-test-tools was used for nodejs v0.10.

  3. Replaces ansi-color and color-suuport to chalk

    Because this package used chalk originally, and chalk enables to drop the codes for --color/--no-color flags.

  4. Drops isobject and array-sort

    By adding alternative codes.

  5. Drops old gulp supports

    This PR removes ^3.7.0, ^4.0.0-alpha.1, and ^4.0.0-alpha.2 directories.

  6. Change directory configuration.

@sttk sttk requested a review from phated November 30, 2022 13:57
@sttk sttk changed the title chore: Normalize repository, dropping node <10.13 support chore!: Normalize repository, dropping node <10.13 support Nov 30, 2022
@sttk
Copy link
Contributor Author

sttk commented Nov 30, 2022

I'll make a PR for using theming-log after this PR.

@phated
Copy link
Member

phated commented Dec 9, 2022

@sttk why did you remove support for gulp 3.x? This package is supposed to support all versions of gulp we can.

@sttk
Copy link
Contributor Author

sttk commented Dec 11, 2022

@phated Since gulp-cli is added dependencies of gulp, gulp-cli is not needed to install globally and we can think that new gulp-cli is used in new gulp.
In addition, it's difficult that we'll continue to support gulp v3 on new node and new versions of dependencies.
So I think it's better to remove supporting gulp v3.

@sttk
Copy link
Contributor Author

sttk commented Dec 11, 2022

@phated I've modified Liftoff.configFiles as I wrote in liftoff#126.

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Since gulp-cli is added dependencies of gulp, gulp-cli is not needed to install globally and we can think that new gulp-cli is used in new gulp.

The purpose of this is to allow the node_modules/.bin/gulp to be placed when having the gulp dependency, but we don't want users to be installing gulp globally. Instead the gulp-cli project should be installed globally and needs to support each version of gulp.

In addition, it's difficult that we'll continue to support gulp v3 on new node and new versions of dependencies.

We can update dependencies and node versions while still supporting gulp 3 and gulp 4. The versioned directory allows for us to have isolated code for each one. Please restore the gulp 3 support in this PR.

Additionally, I was trying to review this PR but it seems like you renamed and moved files around without much purpose. Can you reduce this PR to the dependency upgrades/changes and normalization for an easier review? We can add follow up PRs that refactor.

LICENSE Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@@ -114,7 +114,7 @@ Supported configurations properties:
| flags.gulpfile | Set a default gulpfile |
| flags.silent | Silence logging by default |
| flags.series | Run tasks given on the CLI in series (the default is parallel) |
| flags.require | An array of modules to require before running the gulpfile. Any relative paths will be resolved against the `--cwd` directory (if you don't want that behavior, use absolute paths) |
| flags.preload | An array of modules to preload before running the gulpfile. Any relative paths will be resolved against the `--cwd` directory (if you don't want that behavior, use absolute paths) |
Copy link
Member

Choose a reason for hiding this comment

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

Note to self: Include this rename in the conventional commits so it shows in changelog

README.md Outdated Show resolved Hide resolved
@sttk
Copy link
Contributor Author

sttk commented Sep 10, 2023

@phated I've reverted the two commits about dropping old gulp supports and changing directory configuration. Please review this PR again.

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Thanks @sttk! This is excellent work. I had a few questions and comments that I'd like addressed.

LICENSE Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
index.js Outdated Show resolved Hide resolved
lib/shared/options/parser.js Show resolved Hide resolved
test/lib/merge-configs.js Outdated Show resolved Hide resolved
lib/shared/config/merge-configs.js Outdated Show resolved Hide resolved
test/sync-task.js Outdated Show resolved Hide resolved
test/sync-task.js Outdated Show resolved Hide resolved
test/sync-task.js Outdated Show resolved Hide resolved
Co-authored-by: Blaine Bublitz <blaine.bublitz@gmail.com>
@sttk
Copy link
Contributor Author

sttk commented Oct 8, 2023

@phated I've done except the matter regarding archy, pretty-hrtime, and merging config by mergeConfigToCliFlags, mergeConfigToEnvFlags. It will take a little more time for these remaining matter, so please wait for a while.

@sttk
Copy link
Contributor Author

sttk commented Nov 19, 2023

@phated All changes have been completed. Please review this PR again. 🙇

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Thanks for the changes here @sttk! I have a few more comments, but this is nearly completed 🎉

lib/shared/verify-dependencies.js Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
lib/shared/log/tasks.js Outdated Show resolved Hide resolved
lib/shared/log/tasks.js Outdated Show resolved Hide resolved
test/config-description.js Outdated Show resolved Hide resolved
test/config-flags-gulpfile.js Outdated Show resolved Hide resolved
test/flags-verify.js Show resolved Hide resolved
test/tool/gulp-cmd.js Outdated Show resolved Hide resolved
@sttk
Copy link
Contributor Author

sttk commented Jan 6, 2024

@phated I've modified what you pointed out. Please review again. 🙇

Copy link
Member

@phated phated left a comment

Choose a reason for hiding this comment

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

Massive thank you @sttk for all your work here! This PR turned out excellent. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
v5
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants