Skip to content
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

fix: changes findAll to only run on plugindata nodes, except for inspect #2163

Merged
merged 3 commits into from
Aug 14, 2023

Conversation

six7
Copy link
Collaborator

@six7 six7 commented Aug 14, 2023

introduces performance optimization for the findAll function utilizing Figma's new addition to their plugin API, with the exception for the Inspect where we still want to show all (eg styles or variables)

@changeset-bot
Copy link

changeset-bot bot commented Aug 14, 2023

🦋 Changeset detected

Latest commit: 7f3ed75

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@tokens-studio/figma-plugin Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions
Copy link
Contributor

PR Analysis

  • 🎯 Main theme: Performance optimization for the findAll function
  • 📌 Type of PR: Enhancement
  • Focused PR: True

PR Feedback

  • General suggestions: The PR looks good overall, with a focused theme and clear description. However, it would be beneficial to add relevant tests to ensure the correctness of the performance optimization.

  • 🤖 Code feedback:

    • relevant file: src/plugin/NodeManager.ts
      suggestion: Consider renaming the opts parameter to options for better readability. [medium]
      relevant line: public async findNodesWithData(opts: {

    • relevant file: src/plugin/NodeManager.ts
      suggestion: Instead of using a boolean flag nodesWithoutPluginData, consider using an enum or a more descriptive option name to improve code clarity. [medium]
      relevant line: nodesWithoutPluginData?: boolean;

    • relevant file: src/utils/findAll.ts
      suggestion: Consider extracting the plugin data options into a separate function for better modularity and reusability. [medium]
      relevant line: const pluginDataOptions = nodesWithoutPluginData

How to use

To invoke the PR-Agent, add a comment using one of the following commands:
/review [-i]: Request a review of your Pull Request. For an incremental review, which only considers changes since the last review, include the '-i' option.
/describe: Modify the PR title and description based on the contents of the PR.
/improve: Suggest improvements to the code in the PR.
/ask <QUESTION>: Pose a question about the PR.

To edit any configuration parameter from 'configuration.toml', add --config_path=new_value
For example: /review --pr_reviewer.extra_instructions="focus on the file: ..."
To list the possible configuration parameters, use the /config command.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 14, 2023

Commit SHA:493ccdb6f5551cd2ddaba79ed588a449493696c7

Test coverage results 🧪

Code coverage diff between base branch:main and head branch: fix/selection-faster 
Status File % Stmts % Branch % Funcs % Lines
🔴 total 68.3 (-0.01) 59.58 (-0.03) 66.55 (0) 68.57 (0)

@github-actions
Copy link
Contributor

github-actions bot commented Aug 14, 2023

Commit SHA:493ccdb6f5551cd2ddaba79ed588a449493696c7
Current PR reduces the test coverage percentage by 100 for some tests

@six7 six7 merged commit b5e7b88 into main Aug 14, 2023
5 of 6 checks passed
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