Move LoadingCountService into a subset of HttpService #31800
Conversation
💚 Build Succeeded |
| @@ -59,16 +59,15 @@ const MockUiSettingsClient = mockClass('./ui_settings_client', UiSettingsClient, | |||
| // Load the service | |||
| import { UiSettingsService } from './ui_settings_service'; | |||
|
|
|||
| const loadingCountStartContract = { | |||
| loadingCountStartContract: true, | |||
There was a problem hiding this comment.
Doesn't seem like loadingCountStartContract: true did anything, so I removed it.
💚 Build Succeeded |
rudolf
added
the
Team:Core
|
Pinging @elastic/kibana-platform |
epixa
added
v8.0.0
v7.2.0
non-issue
There was a problem hiding this comment.
LGTM
I think ultimately we'll want to remove the addLoaderCount public API and deal with the counter entirely internal to the HTTP service, but we can only really do that when all HTTP requests are running through this service, and we're not anywhere close to there yet.
| @@ -1,38 +0,0 @@ | |||
| // Jest Snapshot v1, https://goo.gl/fbAQLP | |||
There was a problem hiding this comment.
optional nit IMHO it's crucial to keep git story clean in a big project because often we need to find when and why something was added in the codebase. so I'd use git mv here
There was a problem hiding this comment.
@restrry I totally agree. I think this is a limitation in the github PR changes view. The commit (5f171bd) shows the rename, but because a subsequent commit rewrote so much of the file, the summary view makes it appear as if a new file was created.
Will rename loading_count_service.ts in a separate commit.
rudolf
force-pushed
the
move-loadingcount-to-http-service
branch
from
2d4ab0a to
0fae1d8
Compare
💚 Build Succeeded |
* LoadingCountService is now a subset of HttpService Will rename loading_count_service.ts in a separate commit. * Rename files/folders from loading_count -> http * Reorganize public HttpService tests
Summary
Instead of having a service dedicated to this (
LoadingCountService), keeping track of loading counts is now part of the publicHttpService.Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.This was checked for cross-browser compatibility, including a check against IE11Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n supportDocumentation was added for features that require explanation or tutorialsThis was checked for keyboard-only and screenreader accessibilityFor maintainers
This includes a feature addition or change that requires a release note and was labeled appropriately