Skip to content

Improve error message #10

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

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Improve error message #10

wants to merge 3 commits into from

Conversation

thet
Copy link
Member

@thet thet commented Apr 25, 2025

UPDATE @rnixx @lenadax

I‌ changed the PR in CMFPlone to not belong anymore on this PR.
The callback isn't necessary anymore!

In this case, I'd even remove the callback commit.

However, at least the change which handles resolver errors gracefully is useful[

What do you think?

Belongs together:

Dependency problem with resources without these fixes or by just using webresource.GracefulResourceRenderer:

image

Dependency problem with the two PRs in place:

image

Note: The circular dependency problem seems to be incorrect.

@thet thet marked this pull request as draft April 25, 2025 01:05
@rnixx
Copy link
Member

rnixx commented Apr 25, 2025

Thank you. Can you add tests for your changes?

@thet
Copy link
Member Author

thet commented May 21, 2025

I now barely understand why I did the things as I did them... I need definitely re-think this and add tests to get this code nice and tidy.
Will do ASAP, but first I have to finish some urgent stuff.

@thet thet force-pushed the improve-error-message branch 3 times, most recently from 3355235 to 832c727 Compare May 26, 2025 22:58
@thet thet force-pushed the improve-error-message branch from 832c727 to 7ec5ad3 Compare May 27, 2025 09:38
thet added 2 commits May 27, 2025 11:43
This allows to run some code in an error case, e.g. to add a user-visible
status message.
@thet
Copy link
Member Author

thet commented May 27, 2025

I updated the code, made it more clear and added tests.

Ready to review...

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