| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
There was a problem hiding this comment.
Implementation looks really nice to me. Left a couple of comments but generally looks really good
Sorry, something went wrong.
|
Rob Holt (@rjmholt) Thanks for the review and feedback! I addressed them all. Also, I decided to use using declaration in QueryNewReleaseAsync to make it less nested. |
Sorry, something went wrong.
|
Joey Aiello (@joeyaiello) Could you please take a look at the current notification message in the PR description? I decided to not print it in color as it's (1) over striking (annoying) (2) not accessibility friendly. |
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM!
Sorry, something went wrong.
|
Rebased to resolve a conflict in experimental feature declaration. |
Sorry, something went wrong.
|
No tests was added. |
Sorry, something went wrong.
|
Ilya (@iSazonov) I don't know how to add a test for it. It seems manually testable only -- only interactive session will trigger update-check and notification-printing. |
Sorry, something went wrong.
|
We could use HttpListener helper module to emulate server responses, HelpersHostCS to get host output, and test hook to force update check. Although I'm not sure it is worth the effort. Maybe after it ceases to be experimental. |
Sorry, something went wrong.
|
Dongbo Wang (@daxian-dbw) |
Sorry, something went wrong.
|
Christoph Bergmeister (@bergmeister) I'm going to do an update to the message. Let me look at some examples from other software. |
Sorry, something went wrong.
|
Christoph Bergmeister (@bergmeister) This is an experimental feature and feedbacks is not a late. And we were more worried about the code than about the look. :-) |
Sorry, something went wrong.
Not clear how. |
Sorry, something went wrong.
|
Christoph Bergmeister (@bergmeister) Thanks for the feedback! I was expecting Joey Aiello (@joeyaiello) to take a look at the message but he was busy and didn't get to it. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
RFC: PowerShell/PowerShell-RFC#162
Support notification on pwsh startup when a new release is available
For detailed implementation design, please look at the RFC.
Notification Message
For stable version pwsh:
For preview version pwsh:
Startup Perf Measurement on my dev machines
Windows
When no new release is available, namely on the latest preview/stable pwsh where notification will not be printed, the update notification change adds about 4ms to the startup time of an interactive session.
When a new release is available, namely on an old preview/stable pwsh where a notification message will be printed, the update notification change adds about 6ms to the startup time of an interactive session.
As a reference, Measure-Command { F:\pscore70.preview4\pwsh.exe -noprofile -c exit } takes about 400ms on average on my Windows dev machine.
Linux (Ubuntu 16.04)
When no new release is available, namely on the latest preview/stable pwsh where notification will not be printed, the update notification change adds about 6ms to the startup time of an interactive session.
When a new release is available, namely on an old preview/stable pwsh where a notification message will be printed, the update notification change adds about 9ms to the startup time of an interactive session.
As a reference, Measure-Command { pwsh-preview -noprofile -c exit } takes about 445ms on average on my Linux dev machine.
macOS (10.14.6)
When no new release is available, namely on the latest preview/stable pwsh where notification will not be printed, the update notification change adds about 4ms to the startup time of an interactive session.
When a new release is available, namely on an old preview/stable pwsh where a notification message will be printed, the update notification change adds about 5ms to the startup time of an interactive session.
As a reference, Measure-Command { pwsh-preview -noprofile -c exit } takes about 316ms on average on my macOS dev machine.
PR Checklist