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

Nicer error message for duplicate deps #158

Merged
merged 1 commit into from
Aug 9, 2024
Merged

Conversation

nhomble
Copy link
Contributor

@nhomble nhomble commented Aug 7, 2024

Follow up to: #156

I apologize @vuki656, I was poking around for another I was going to propose when I noticed this highlight config. Some colors/icons seem like better feedback rather than [ERROR].

Here's a proposal pr to incorporate .in_error dependencies to the state object. Let me know what you think and how I can polish it!

Copy link
Owner

@vuki656 vuki656 left a comment

Choose a reason for hiding this comment

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

Hey. Sounds good. I'd just change in_error to invalid.

@vuki656 vuki656 changed the title highlights errors Nicer error message for duplicate deps Aug 7, 2024
@nhomble nhomble requested a review from vuki656 August 8, 2024 06:00
@@ -21,6 +21,11 @@ M.dependencies = {
-- current: string - current dependency version
-- }
installed = {},
-- dependencies that we were able to parse but something is wrong about them
Copy link
Owner

Choose a reason for hiding this comment

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

You can remove this if it's not needed

@vuki656
Copy link
Owner

vuki656 commented Aug 8, 2024

Can you also add it to README?

Otherwise looks good. I'll merge after

@nhomble nhomble requested a review from vuki656 August 9, 2024 05:33
@vuki656 vuki656 merged commit a3a7573 into vuki656:master Aug 9, 2024
4 checks passed
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