-
Notifications
You must be signed in to change notification settings - Fork 67
chore: Inline the remaining files #1470
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
Conversation
@@ -56,7 +54,7 @@ export class Doctor { | |||
|
|||
async diagnose(): Promise<DiagnosticInfo> { | |||
SpinniesManager.add('runningDiagnostics', { | |||
text: i18n(`${i18nKey}.runningDiagnostics`), | |||
text: i18n(`lib.doctor.runningDiagnostics`), |
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.
Cursor seemed to have a hard time with this file. It wanted to inline them like this: ${'lib.doctor'}.runningDiagnostics
, so I would pay extra special attention to this file.
@@ -13,29 +11,29 @@ type CreateFunctionPromptResponse = { | |||
|
|||
const FUNCTIONS_FOLDER_PROMPT: PromptConfig<CreateFunctionPromptResponse> = { | |||
name: 'functionsFolder', | |||
message: i18n(`${i18nKey}.enterFolder`), | |||
message: i18n(`lib.prompts.createFunctionPrompt.enterFolder`), | |||
validate(val?: string) { | |||
if (typeof val !== 'string') { |
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.
This is outside the scope of this prompt, but we repeat the same validate function three times with the same i18n strings--can we combine into one function?
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'm going to leave that alone for a later refactoring. I want this PR to focus solely on inlining the strings and not refactoring anything. These are already really large PRs so I'm trying to limit risk as much as possible and keep them targeted and easy to review.
@@ -28,7 +28,7 @@ const DEFAULT_TABLE_HEADER = [ | |||
]; | |||
|
|||
exports.command = 'lighthouse-score [--theme]'; | |||
exports.describe = false; // i18n(`${i18nKey}.describe`); | |||
exports.describe = false; |
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 think we can unhide this command now. Maybe check with @brandenrodgers
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.
The last time this came up, the answer was to let the CMS team determine the future of the commands. I'm not sure where they landed on it
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.
Some minor comments but otherwise, looks good to me!
Description and Context
In the remaining keys in preparation for the switch