-
Notifications
You must be signed in to change notification settings - Fork 28
Fix plots file watchers (diff output key is not always a file) #2854
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
d790043 to
5f2f179
Compare
5f2f179 to
b72f392
Compare
|
|
||
| abstract managedUpdate(path?: string): Promise<unknown> | ||
|
|
||
| protected abstract collectFiles(data: T): string[] |
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.
[F] TIL this is possible.
| ...args | ||
| ) | ||
|
|
||
| this.notifyChanged({ data, revs }) |
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.
[F] We don't want to delay updating the rest of the app whilst we mess around looking for files to watch.
8e422e0 to
fbfff9a
Compare
|
I had to update the test fixture because the Windows tests were failing. |
fbfff9a to
42f86d8
Compare
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.
The PR diff size of 30454 lines exceeds the maximum allowed for the inline comments feature.
|
Code Climate has analyzed commit 42f86d8 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 96.9% (85% is the threshold). This pull request will bring the total coverage in the repository to 96.6%. View more on Code Climate. |
2/2
main<- #2852 <- thisCloses #2851.
☝🏻 I actually remembered correctly.
Unfortunately, I realised that the key can actually be the plot's title which means we need to interrogate the data 😢.