-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
namespaced kit options #236
Conversation
|
Renamed |
export function validate_config(config) { | ||
for (const key in config) { | ||
if (!expected.has(key)) { | ||
warn(`Unexpected option ${key}${key in options ? ` (did you mean kit.${key}?)` : ''}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think the 'did you mean' here is going to be generally helpful to future users? Singling this out seems to be maybe a bit too focused on people who used Kit before this change, which is practically nobody.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's really just there to reduce the number of discord DMs/mentions i/we get in the near term. agree that it doesn't serve much long term purpose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I somehow had forgotten that there are people out there using it who don't have access to this repo. In light of that, I'd say that yes this warning was a good idea.
See #229 (comment), though maybe should be
kit
rather thankitOptions
.Would also like to get some validation in here before this gets merged