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

Update tsconfig.json validation messages #2322

Merged
merged 3 commits into from
Oct 28, 2024
Merged

Conversation

infomiho
Copy link
Contributor

@infomiho infomiho commented Oct 4, 2024

We didn't output the fully qualified field name for the missing error message e.g. we sad "module" is missing vs. "compilerOptions.module" is missing.

This is maybe a bit over engineered, but I thought it looked nicely expressed with the type.

@infomiho infomiho requested a review from Martinsos October 15, 2024 13:21
Copy link
Member

@Martinsos Martinsos left a comment

Choose a reason for hiding this comment

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

I like it, LGTM! One small potential fix.

type FieldPath = [String]

instance Show FullyQualifiedFieldName where
show (FieldName fieldPath) = show $ intercalate "." fieldPath
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
show (FieldName fieldPath) = show $ intercalate "." fieldPath
show (FieldName fieldPath) = intercalate "." fieldPath

I think this extra show was redundant -> intercalate will already return a string in this case!

If you do wnat to have extra set of quotes around it, I would that below in the place of call, not add them here with show. Although I guess it was just a leftover.

@infomiho infomiho merged commit c350cbe into main Oct 28, 2024
6 checks passed
@infomiho infomiho deleted the miho-validation-message branch October 28, 2024 23:17
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