Skip to content

Conversation

@esezen
Copy link
Contributor

@esezen esezen commented Jun 9, 2023

Resolves security warnings caused by us and also simplifies description generation

@esezen esezen added the wip Work in progress label Jun 9, 2023
@esezen esezen requested review from a team June 12, 2023 15:25
@esezen esezen removed the wip Work in progress label Jun 12, 2023
Copy link
Contributor

@mocca102 mocca102 left a comment

Choose a reason for hiding this comment

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

It's looking good except for the places where defaultArgumentsCode that got removed was used


export const defaultOnSubmitCode = `"onSubmit": (submitEvent) => console.dir(submitEvent)`;

export const defaultArgumentsCode = (apiKey: string) => `"apiKey": "${apiKey}",
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 used here and here

I can remove them and update the args if you'd like to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! Fixed here 13dab01

);

// Replace template blocks with function strings
Array.from(res.matchAll(/"(\w*)_CODE"/g)).forEach((match) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Big brains

@esezen esezen requested a review from mocca102 June 15, 2023 16:52
Copy link
Contributor

@mocca102 mocca102 left a comment

Choose a reason for hiding this comment

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

LGTM!

@esezen esezen merged commit 2d61c7f into main Jun 15, 2023
@esezen esezen deleted the csl-2463-resolve-code-scanning-warnings branch June 15, 2023 18:19
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