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: Pass in markdown renderer to speed up command #313

Merged
merged 1 commit into from
May 31, 2024

Conversation

dbolson
Copy link
Contributor

@dbolson dbolson commented May 31, 2024

Creating a new renderer for each generated resource action is slow, so we can create a single one and pass it into each function that needs it. This reduces the call to AddAllResourceCmds from over five seconds to a few hundred ms.

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

Provide links to any issues in this repository or elsewhere relating to this pull request.

Describe the solution you've provided

Provide a clear and concise description of what you expect to happen.

Describe alternatives you've considered

Provide a clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context about the pull request here.

@@ -14,270 +15,320 @@ func AddAllResourceCmds(
client resources.Client,
analyticsTrackerFn analytics.TrackerFn,
) {
markdownRenderer, _ := glamour.NewTermRenderer(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is where we create the single renderer.

@dbolson dbolson requested a review from k3llymariee May 31, 2024 15:49
@dbolson dbolson merged commit 699a980 into main May 31, 2024
2 checks passed
@dbolson dbolson deleted the sc-246033/speed-up-markdown branch May 31, 2024 21:29
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