Skip to content

Conversation

@joe-yeager
Copy link
Contributor

Description and Context

In the remaining keys in preparation for the switch

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.

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.


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.

3 participants