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

improve: create missing RendererFunc error message #166

Closed
wants to merge 1 commit into from
Closed

improve: create missing RendererFunc error message #166

wants to merge 1 commit into from

Conversation

13rac1
Copy link

@13rac1 13rac1 commented Dec 12, 2020

Improve the development debugging process by better handling a
missing Node Kind RendererFunc. This:

panic: runtime error: index out of range [22] with length 20

Becomes:

panic: RendererFunc not found for kind: KindName

Improve the development debugging process by better handling a
missing Node Kind RendererFunc. This:
panic: runtime error: index out of range [22] with length 20
Becomes:
panic: RendererFunc not found for kind: KindName
@13rac1 13rac1 changed the title Improve missing RendererFunc error message improve: create missing RendererFunc error message Dec 27, 2020
@13rac1
Copy link
Author

13rac1 commented Jan 10, 2021

@yuin Is there any issue with this change? It's been sitting a month now.

@13rac1
Copy link
Author

13rac1 commented Feb 9, 2021

Bumping to avoid Bot closure.

@stale
Copy link

stale bot commented Mar 16, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 16, 2021
@13rac1
Copy link
Author

13rac1 commented Mar 16, 2021

@Stale No.

@stale stale bot removed the stale label Mar 16, 2021
@yuin
Copy link
Owner

yuin commented Mar 26, 2021

Same PR was submitted before: #107.
If I merge this kind of PR, I prioritize older PR over this.

@yuin yuin closed this Mar 26, 2021
@13rac1
Copy link
Author

13rac1 commented Mar 26, 2021

In #107 (comment)

This kind of PR has been submitted before.
Works as intended. This is coding problem so it should panic.

I agree, it should panic, which is why my implementation continues to do so, but the message is now useful/helpful and not incorrect/misleading for the developer.

If I merge this kind of PR, I prioritize older PR over this.

Okay, can we merge #110 ? The fact @abhinav, someone else, and myself have all attempted to correct this developer experience speaks to the frustration encountered.

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.

None yet

2 participants