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

Replace colors by ansis #17

Closed
wants to merge 3 commits into from

Conversation

KristjanESPERANTO
Copy link

You are using the colors package that is installed by the MagicMirror² core. We are about to remove it from the core, then your module would have a problem with this dependency starting with the next MagicMirror² release.

Instead of including colors into your dependencies, I think it would be a better approach to replace it with the drop-in replacement ansis (as we about to do in core).

I had problems getting your build process to run, so I haven't tested my changes. But I am positive that it will work 🙂

In the second commit I also made a few cosmetic improvements.

@djey47
Copy link
Owner

djey47 commented Jan 15, 2024

Hello, colors is only used within a 'mock' server faking some APIs for development purposes.

I guess I should have added colors to my own package.json at the first place. It just worked as MM2 is providing it, I wasn't aware of it.

Okay to use ansis as alternative.

@KristjanESPERANTO
Copy link
Author

I guess I should have added colors to my own package.json at the first place.

Exactly 🙂

I've searched through all the modules on the official list so that we don't break any modules with this change for the next release.

@djey47
Copy link
Owner

djey47 commented Jan 15, 2024

Thanks. But are you doing this for each one of the MM2 modules? That's quite heavy work !

And concerning the CI, indeed the master branch has issues breaking the workflow (unit tests), it should be sorted out after merging my current dev branch (transilien) which is running well.

@KristjanESPERANTO
Copy link
Author

Thanks. But are you doing this for each one of the MM2 modules? That's quite heavy work !

I am in the process of building a new list of 3rd party modules based on the official list. I'm already checking a few rudimentary things on all modules. So I just used this to search for require('colors 😄

@KristjanESPERANTO
Copy link
Author

I'm not that familiar with your build process, so I don't know why your tests are failing. Can you handle it? Otherwise we can just add colors to your package.json.

@djey47
Copy link
Owner

djey47 commented Jan 15, 2024

The issue might be related to old libraries I've removed by changing the APIs I'm using. It is absolutely not related to the removal of colors as it already failed before.

I'm positive that merging my dev branch will eventually get everything running great with the latest changes. Btw, I will merge upcoming master into it to check.

Anayway, thanks for your help proposal.

@djey47
Copy link
Owner

djey47 commented Mar 2, 2024

Created a new PR with the same changes after latest master update: #18

@djey47 djey47 closed this Mar 2, 2024
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