Skip to content

refactor: replace cli-color with picocolors #1532

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

JounQin
Copy link

@JounQin JounQin commented Apr 15, 2025

Pull Request check-list

Please make sure to review and check all of these items:

  • Does npm run test pass with this change (including linting)?
  • Does the description below contain a link to an existing issue (Closes #[issue]) or a description of the issue you are solving?
  • Have you added new tests to prevent regressions?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

Description of change

cli-color -> es5-ext detected as a virus

medikoo/es5-ext#186

And picocolors has much lighter weight, no dependencies.

@JounQin
Copy link
Author

JounQin commented Apr 15, 2025

The CI error should be unrelated:

  docker-compose up -d ${DIALECT}
  shell: /usr/bin/bash -e {0}
  env:
    DIALECT: postgres
    SEQ_PORT: 54320
/home/runner/work/_temp/4024a7e3-607c-4ffe-b82d-9ae14520d99c.sh: line 1: docker-compose: command not found
Error: Process completed with exit code 127.

@WikiRik
Copy link
Member

WikiRik commented Apr 15, 2025

CI seems to be broken atm. It uses ubuntu-latest and it wasn't written for Ubuntu 24.04 and the new Docker version. It should use docker compose (space instead of -). That should be fixed as well before we can merge this.

Even with that; in the new version of the CLI we're using ansis. Can we use that here as well?
https://github.com/sequelize/sequelize/blob/2a6590352e6f3d31f15bddf54a9a8a9f83312600/packages/cli/package.json#L14

@JounQin
Copy link
Author

JounQin commented Apr 15, 2025

CI seems to be broken atm. It uses ubuntu-latest and it wasn't written for Ubuntu 24.04 and the new Docker version. It should use docker compose (space instead of -). That should be fixed as well before we can merge this

I'm not too familiar with the codebase yet, where should I change?

Even with that; in the new version of the CLI we're using ansis.

Didn't notice that, I'll try to replace ansis at the same time. By the way, why there're two libraries for cli coloring in the first place? 😅

@WikiRik
Copy link
Member

WikiRik commented Apr 16, 2025

The CI for PRs is here; https://github.com/sequelize/cli/blob/main/.github%2Fworkflows%2Fci.yml

With the new version of the CLI, I meant the rewrite that we are doing over in the sequelize/sequelize repo for v7. This sequelize-cli library will be deprecated once that comes out.

@JounQin
Copy link
Author

JounQin commented Apr 16, 2025

The CI for PRs is here; main/.github%2Fworkflows%2Fci.yml

With the new version of the CLI, I meant the rewrite that we are doing over in the sequelize/sequelize repo for v7. This sequelize-cli library will be deprecated once that comes out.

I already tried at 433846a (#1532)

And there is a new error:

Run docker run --link ${DIALECT}:db -e CHECK_PORT=${SEQ_PORT::-1} -e CHECK_HOST=db --net cli_default giorgos/takis
  docker run --link ${DIALECT}:db -e CHECK_PORT=${SEQ_PORT::-1} -e CHECK_HOST=db --net cli_default giorgos/takis
  shell: /usr/bin/bash -e {0}
  env:
    DIALECT: mysql
    SEQ_PORT: 33060
Unable to find image 'giorgos/takis:latest' locally
latest: Pulling from giorgos/takis
docker: [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/giorgos/takis:latest to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/.
See 'docker run --help'.
Error: Process completed with exit code 125.

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