-
Notifications
You must be signed in to change notification settings - Fork 119
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
Feat(compiler-base): add error message. #164
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
add error message loader based on fluent0.16.0. issue #115
Pull Request Test Coverage Report for Build 2933389271
💛 - Coveralls |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Three important things:
- It is recommended to use
anyhow::Result
as a function return instead of.unwrap_or_else(|| bug!("error message"))
Because it's not actually an internal compiler error but a user error. - Whether it is a high-level or medium-level API, try to keep the interface as simple as possible, such as importing only one file and using one structure.
- Try the
once_cell
crate to initTempateMessageLoader
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGM
1. Does this PR affect any open issues?(Y/N) and add issue references (e.g. "fix #123", "re #123".):
issue #115
2. What is the scope of this PR (e.g. component or file name):
compiler_base/error_message.rs
3. Provide a description of the PR(e.g. more details, effects, motivations or doc link):
add
ErrorMessage
to define the message displayed in diagnostics.add
TemplateLoader
to load text template file ("*.ftl").4. Are there any breaking changes?(Y/N) and describe the breaking changes(e.g. more details, motivations or doc link):
5. Are there test cases for these changes?(Y/N) select and add more details, references or doc links:
add unit test in compiler_base/tests.rs
6. Release note
Please refer to Release Notes Language Style Guide to write a quality release note.