-
Notifications
You must be signed in to change notification settings - Fork 5
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
ADRs: 012-016 - Adding Impact Sections #1627
base: main
Are you sure you want to change the base?
Conversation
- Filled in Impact section - updated call to OeA adr
Filled in impact sections for editing by team
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
||
### Positive | ||
|
||
- **Enhanced Security:** AuthN/AuthZ ensures that only authorized users can access specific resources, reducing the risk of unauthorized access. |
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.
Consider reducing the verbosity of the Impact section by summarizing the key points more succinctly. This will help maintain the readability and focus of the document. [important]
|
||
### Positive | ||
|
||
- **Decoupling of Business Logic and Third-Party Libraries:** allows us to decouple our business logic from the specific implementation details of the third-party library. This enhances maintainability and readability. |
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.
It might be beneficial to add specific examples or case studies that demonstrate the positive impacts mentioned, such as how the decoupling has concretely improved maintainability in past projects. [medium]
|
||
### Risks | ||
|
||
- **Human Error in Implementation:** Incorrectly applying the naming convention during key creation or migration could lead to confusion or outages. |
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.
To address the risk of human error in implementation, consider recommending specific tools or automated checks that can enforce the naming conventions consistently across the project. [important]
PR Code Suggestions ✨Explore these optional code suggestions:
|
Quality Gate passedIssues Measures |
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.
Is the FHIR library
the only wrapped domain object currently?
Description
This PR covers filling in the impact sections for ADRs 012-016 so that it is understood why the decision was made at the time and what the concerns were.
This PR errs on the side of being more verbose than not so that unneeded items can be stripped away during review, so please read carefully.
NOTE: a change to ADR 001 was added to this PR to make linking work better in the IDE.
Issue #1247
Checklist