Notifications: improve copy on error messages #11062
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This validation should eventually happen at the Read the Docs config file. We are not there, but at least we are giving a better error message to users. Closes #8352
agjohnson
approved these changes
Jan 24, 2024
There was a problem hiding this comment.
Thanks for going through these! I've tuned the copy more with additions. A few broad notes:
- Some of the headers might still be tuned. For example, I noted some headers were
Invalid configurationwhile the error noted missing values, and a more specific header could be used instead. - We have a fair amount of existing copy that interchangeable used
config fileandconfiguration file, I think the latter is preferred for clarity - There are some conflicts between the terms
configandoption. To me,optionis a single key in a configuration file andconfigurationrefers to the whole file.
Co-authored-by: Anthony <[email protected]>
|
@agjohnson thanks for the review! I took most of your suggestions 💯 . There are a few that aren't marked as resolved. Please, take a final look at them if you have time. |
|
Getting the classname doesn't work on templates, tho: 🤔 |
``` In [4]: n.get_message().get_rendered_body() Out[4]: '\n\n\nConfig validation error in <code></code>.\nExpected a list, got type <code>int</code> (<code>422</code>).' ```
|
I create a simple template filter to be able to render the class name for now: |
|
Yeah, I thought the same here as you. Annoying that underscore attributes are off limits, but I'm not surprised Django templates get in our way here. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is not perfect and there is still more work to do here. However, this is another good iteration in the process.
Closes #10985