-
Notifications
You must be signed in to change notification settings - Fork 475
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
WIP: Care Plugin Support #8660
WIP: Care Plugin Support #8660
Conversation
❌ Deploy Preview for care-ohc failed.
|
CARE Run #3681
Run Properties:
|
Project |
CARE
|
Run status |
Passed #3681
|
Run duration | 03m 29s |
Commit |
e37f9af8e9: WIP: Care Plugin Support
|
Committer | Gigin George |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
125
|
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.
- Everything in
src/Routers/routes/
can be merged early as another PR to reduce clutter in this PR. - Can you push an app repo, I am unable to review the config bit as I don't know the shape of manifest.
cc: @shivankacker
src/PluginEngine.tsx
Outdated
type SupportedPluginExtensions = | ||
| "DoctorConnectButtons" | ||
| "PatientExternalRegistration"; |
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.
Not sure about this, can't we do something like route
and id
?
This is quite restricting, we need to whitelist all locations. 🤔 (Just a thought)
👋 Hi, @gigincg, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
@gigincg can you update the status here? |
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.
Need to update README to instruct how to install plugins (npm run setup
and first time setup npm run install-all
)
👋 Hi, @gigincg, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
…g logic to support branches.
👋 Hi, @gigincg, This message is automatically generated by prince-chrismc/label-merge-conflicts-action so don't hesitate to report issues/improvements there. |
@gigincg Your efforts have helped advance digital healthcare and TeleICU systems. 🚀 Thank you for taking the time out to make CARE better. We hope you continue to innovate and contribute; your impact is immense! 🙌 |
Proposed Changes
@ohcnetwork/care-fe-code-reviewers
Merge Checklist
Beside
npm i
we need to usenpm run install-all
beforenpm run dev
command : Update it in the readme