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.

Sign up for GitHub

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

Jump to bottom

fix(provider-generator): make sanitize comments function robust against diverse inputs #3131

Merged
merged 9 commits into from
Sep 18, 2023
Merged

fix(provider-generator): make sanitize comments function robust against diverse inputs #3131

merged 9 commits into from
Sep 18, 2023

Conversation

DanielMSchmidt
Copy link
Contributor

Closes #3128

@DanielMSchmidt DanielMSchmidt requested a review from a team as a code owner September 14, 2023 08:07
@DanielMSchmidt DanielMSchmidt requested review from ansgarm and Maed223 and removed request for a team September 14, 2023 08:07
ansgarm
ansgarm reviewed Sep 18, 2023
View reviewed changes
Copy link
Member

@ansgarm ansgarm left a comment •
edited

Choose a reason for hiding this comment

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

Looks good, although the added * in front of lines of the comments (as seen in tests) seems to only look nicer 😄

Can we add a test case that actually contains the closing directive */ that failed #3128? Our closest only has /* and I'd like to be sure that this fixes the problem in #3128

@DanielMSchmidt
Copy link
Contributor Author

@ansgarm I thought we added one when we introduced this functionality originally in #2990 but we didn't. I can add a test case

@DanielMSchmidt
Copy link
Contributor Author

Added a test case 👍

@DanielMSchmidt DanielMSchmidt added this pull request to the merge queue Sep 18, 2023
Merged via the queue into main with commit c11ae2f Sep 18, 2023
44 checks passed
@DanielMSchmidt DanielMSchmidt deleted the fix-comment-escaping-in-modules branch September 18, 2023 14:42
This was referenced Oct 4, 2023
Merged
Closed
Closed
@github-actions
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ansgarm ansgarm ansgarm left review comments

@mutahhir mutahhir mutahhir approved these changes

@Maed223 Maed223 Awaiting requested review from Maed223 Maed223 is a code owner automatically assigned from hashicorp/cdktf

Assignees
No one assigned
Labels
ci/provider-integration ci/run-unit/@cdktf/provider-generator
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

invalid code generate by cdktf get
4 participants
@DanielMSchmidt @mutahhir @ansgarm @team-tf-cdk