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(rosetta): fails on "Debug Failure" #2917

Merged
merged 6 commits into from
Jul 21, 2021
Merged

fix(rosetta): fails on "Debug Failure" #2917

merged 6 commits into from
Jul 21, 2021

Conversation

RomainMuller
Copy link
Contributor

@RomainMuller RomainMuller commented Jul 21, 2021

In cases where a literate source file was missing, the substitution
value was not valid TypeScript, which could cause the comipler to fail
on an opaque error (Debug Failure).

This falls back to the visible code for the snippet instead of inserting a
placeholder, and also makes sure to rescue the Debug Failure where
it is emitted from, that is in the call to
program.getDeclarationDiagnostics.

Both of this changes result in greater chances of transliteration
success.

Related to cdklabs/jsii-docgen#369


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

In cases where a literate source file was missing, the substitution
value was not valid TypeScript, which could cause the comipler to fail
on an opaque error (`Debug Failure`).

This changes the substitution text to make it more clear that an example
could not be rendered because the source for it could not be found, and
also makes sure to rescue the `Debug Failure` where it is emitted from,
that is in the call to `program.getDeclarationDiagnostics`.

Both of this changes result in greater chances of transliteration
success.

Related to cdklabs/jsii-docgen#369
@RomainMuller RomainMuller requested a review from iliapolo July 21, 2021 13:11
@RomainMuller RomainMuller self-assigned this Jul 21, 2021
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jul 21, 2021
@iliapolo iliapolo added the pr/do-not-merge This PR should not be merged at this time. label Jul 21, 2021
Copy link
Contributor

@iliapolo iliapolo left a comment

Choose a reason for hiding this comment

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

Added do-not-merge in case you want to address the comment

@RomainMuller RomainMuller removed the pr/do-not-merge This PR should not be merged at this time. label Jul 21, 2021
@mergify
Copy link
Contributor

mergify bot commented Jul 21, 2021

Thank you for contributing! ❤️ I will now look into making sure the PR is up-to-date, then proceed to try and merge it!

@mergify mergify bot added the pr/ready-to-merge This PR is ready to be merged. label Jul 21, 2021
@mergify mergify bot merged commit f6078ef into main Jul 21, 2021
@mergify mergify bot deleted the rmuller/rosetta-bug branch July 21, 2021 15:26
@mergify
Copy link
Contributor

mergify bot commented Jul 21, 2021

Merging (with squash)...

@mergify mergify bot removed the pr/ready-to-merge This PR is ready to be merged. label Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants