Skip to content

Conversation

uli-f
Copy link
Member

@uli-f uli-f commented Feb 9, 2025

  • replace StringBuffer with StringBuilder for improved performance
  • update dependencies and related test classes accordingly
  • simplified code and removed code smells

…r with StringBuilder for improved performance; update dependencies and related test classes accordingly; simplified code and removed code smells
@uli-f
Copy link
Member Author

uli-f commented Feb 9, 2025

I opened a new PR for this instead of continuing with Egon's #1135. It seemed easier as most of Egon's initial changes need to be reversed. I also might have needed GitHub permissions to Egon's fork of cdk 🤷🏼

So please close #1135 and merge this one 😃

Copy link
Member

@egonw egonw left a comment

Choose a reason for hiding this comment

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

I would suggest to not to rename the class. Even the internal variable name (buffer) does not have to be changed. It only makes the patch much larger. But if @johnmay is okay with it, I will also not block the PR because of that. And I also feel the pain of the work done on the renaming.

… make FormatStringBuilder package-private; update constructors and methods to package-private; remove unused imports for FormatStringBuilder in related classes
@uli-f
Copy link
Member Author

uli-f commented Feb 17, 2025

@johnmay Moved FormatStringBuilder to module io and package io as discussed.

@johnmay johnmay merged commit 132856b into cdk:main Feb 17, 2025
6 of 7 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.

3 participants