-
Notifications
You must be signed in to change notification settings - Fork 26
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
initial commit for 2.0 support #78
Conversation
nkolba
commented
Oct 21, 2022
- added 2.0 API side by side with 1.2
- stubbed out 2.0 in preload
- internalized type dependencies for 1.2
WIP. Ran into problems with type usage conflicts between 1.2 and 2.0. |
Looks really promising nonetheless! Looking forward to chatting about it later |
What he said ☝️ |
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
I like the path aliasing, it makes it so much cleaner
I take it that FDC3Events will be split between 1.2 and 2 in a later iteration?
"@fontsource/roboto": "^4.5.3", | ||
"@mui/icons-material": "^5.4.4", | ||
"@mui/material": "^5.4.4", | ||
"electron-fetch": "^1.7.4", | ||
"electron-updater": "^4.6.5", | ||
"fastify": "^3.27.2", | ||
"fdc3-1.2": "npm:@finos/fdc3@^1.2.0", |
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.
should this be here again?
@@ -1,7 +1,11 @@ | |||
import { View } from './view'; | |||
import { getRuntime } from './index'; | |||
import { BrowserWindow } from 'electron'; | |||
import { FDC3App, IntentInstance, ResolverDetail } from './types/FDC3Data'; | |||
import { |
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 IntentResolver common to 1.2 and 2.0?
@@ -14,7 +14,8 @@ | |||
|
|||
"baseUrl": ".", | |||
"paths": { | |||
"/@/*": ["./src/*"] | |||
"/@/*": ["./src/*"], | |||
"/@main/*": ["../main/src/*"], |
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.
I think it's usual to leave off the / on /@main
, no?