-
-
Notifications
You must be signed in to change notification settings - Fork 58
Getting started window on installation #213
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
Conversation
bruno-garcia
left a comment
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.
This is awesome!!
I know it's a draft to serve as a PoC but some notes to discuss the strategy to trigger it
| var imported = importedAssets.Any(path => path.StartsWith("Packages/")); | ||
| var deleted = deletedAssets.Any(path => path.StartsWith("Packages/")); | ||
|
|
||
| if (imported || deleted) |
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.
Do we want to prompt also when asset deleted? I wonder the chance of false positive 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.
Maybe not a prompt but maybe we should delete the assets we create too? Like link.xml and the options file?
| var listRequest = Client.List(true); | ||
| while (!listRequest.IsCompleted) | ||
| { | ||
| Thread.Sleep(100); |
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.
This might freeze up the editor though, right?
Can we schedule the continuation to run async too? If this was TPL it would be listRequest.ContinuesWith(p => .. but I'm not sure the type here (need to load the editor to do a proper review).
bruno-garcia
left a comment
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.
Could we use Events.registeringPackages += RegisteringPackagesEventHandler; instead?
Seems like this API would be perfect:
Yes, definitely! This is way better. |
Popup after package installation:
We request the user to enter their DSN (maybe even with a link where they'd get it from).
Button to press to activate/enable the SDK & close the window? If no DSN is provided and the window is closed the SDK could just stay silent and disabled?
#skip-changelog