Skip to content

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

Merged
merged 3 commits into from
Apr 30, 2025
Merged

Conversation

joe-yeager
Copy link
Contributor

Description and Context

In the remaining keys in preparation for the switch

@@ -56,7 +54,7 @@ export class Doctor {

async diagnose(): Promise<DiagnosticInfo> {
SpinniesManager.add('runningDiagnostics', {
text: i18n(`${i18nKey}.runningDiagnostics`),
text: i18n(`lib.doctor.runningDiagnostics`),
Copy link
Contributor Author

@joe-yeager joe-yeager Apr 28, 2025

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') {
Copy link
Contributor

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?

Copy link
Contributor Author

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;
Copy link
Contributor

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

Copy link
Contributor Author

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

Copy link
Contributor

@kemmerle kemmerle left a 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!

@joe-yeager joe-yeager merged commit e7900c2 into main Apr 30, 2025
1 check passed
@joe-yeager joe-yeager deleted the jy/inline-remaining-i18nkeys branch April 30, 2025 16:41
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