Skip to content

Conversation

@omesh-omg
Copy link
Contributor

@omesh-omg omesh-omg commented Jul 20, 2025

it think it was turned into comment because of lint changes we did last year in july

@aaronreed708
you may highlight if I am wrong and it was done intentionally may be ?

image

re deploying commented code

resolving issue #1124

@netlify
Copy link

netlify bot commented Jul 20, 2025

Deploy Preview for glistening-gecko-6b417a ready!

Name Link
🔨 Latest commit e7bd82a
🔍 Latest deploy log https://app.netlify.com/projects/glistening-gecko-6b417a/deploys/687d186cd7c94400088b2cf9
😎 Deploy Preview https://deploy-preview-1125--glistening-gecko-6b417a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@omesh-omg omesh-omg requested a review from aaronreed708 July 20, 2025 16:26
@aaronreed708
Copy link
Contributor

Hi @omesh-omg! Thank you for your PR! Sorry that it took so long to reply. I was on vacation two weeks ag and last week I tried to verify how this change affects the linting and I noticed that our linting needed to be migrated from v8 to v9. When I did the migration linting stopped working. Hopfully will get some time this week to figure that out. But definitely appreciate your PR!

Copy link
Contributor

@aaronreed708 aaronreed708 left a comment

Choose a reason for hiding this comment

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

Approved with suggested change. Thanks for your contribution!

//const ds = await themeBuilder.getDesignSystem(name);
//const nds = await ds.copy(dest);
const ds = await themeBuilder.getDesignSystem(name);
const nds = await ds.copy(dest);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you drop assigning the result to nds since it looks like copy just blindly returns the new design system. Or maybe better, add something like:

if (!nds) {
    console.log("failed to copy design system: ", name);
}

in case the copy logic ever changes. Just trying to avoid re-introducing the lint warning since I believe that was the whole point in the original change.

@aaronreed708 aaronreed708 mentioned this pull request Aug 14, 2025
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