Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Upgrade fp-ts to 2.8.6 #83866

Merged
merged 6 commits into from
Nov 26, 2020
Merged

Upgrade fp-ts to 2.8.6 #83866

merged 6 commits into from
Nov 26, 2020

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented Nov 19, 2020 •
edited
Loading

Summary

Upgrade fp-ts to the latest minor 2.8.6

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@rudolf
Copy link
Contributor Author

rudolf commented Nov 20, 2020

@elastic/kibana-alerting-services I'd like to bump this dependency, but alerting's page load bundle increased a lot, could you maybe investigate?

id before after diff errors
alerts 89.4KB 119.3KB +29.9KB ❌ 14.9KB over limit

@rudolf rudolf marked this pull request as ready for review November 20, 2020 09:27
@rudolf rudolf added release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Nov 20, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-core (Team:Core)

@gmmorris gmmorris mentioned this pull request Nov 23, 2020
Closed
@gmmorris
Merge branch 'master' into pr/83866
0745518 
* master: (67 commits)
  [Observability] Load hasData call asynchronously (elastic#80644)
  Implement AnonymousAuthenticationProvider. (elastic#79985)
  Deprecate `visualization:colorMapping` advanced setting (elastic#83372)
  [TSVB] [Rollup] Table tab not working with rollup indexes (elastic#83635)
  Revert "[Search] Search batching using bfetch (elastic#83418)" (elastic#84037)
  skip flaky suite (elastic#83772)
  skip flaky suite (elastic#69849)
  create kbn-legacy-logging package (elastic#77678)
  [Search] Search batching using bfetch (elastic#83418)
  [Security Solution] Refactor Timeline flyout to take a full page (elastic#82033)
  Drop use of console-stamp (elastic#83922)
  skip flaky suite (elastic#84011 , elastic#84012)
  Fixed usage of `isReady` for usage collection of alerts and actions (elastic#83760)
  [maps] support URL drilldowns (elastic#83732)
  Revert "Added default dedupKey value as an {{alertInstanceId}} to provide grouping functionality for PagerDuty incidents. (elastic#83226)"
  [code coverage] Update jest config to collect more data (elastic#83804)
  Added default dedupKey value as an {{alertInstanceId}} to provide grouping functionality for PagerDuty incidents. (elastic#83226)
  [Security Solution] Give notice when endpoint policy is out of date (elastic#83469)
  [Security Solution] Sync url state on any changes to query string (elastic#83314)
  [CI] Initial TeamCity implementation (elastic#81043)
  ...
@gmmorris
reduce import size from io-ts
b3ec956
@rudolf rudolf requested a review from a team as a code owner November 23, 2020 12:23
pmuellr
pmuellr reviewed Nov 23, 2020
View reviewed changes
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except that some "loadAlertState" tests have been deleted - is this function being tested elsewhere, so we're eliminating duplicate tests? If not, it seems like we shouldn't be deleting them, or at least we should create an issue to add them back (in some form) once this PR is merged. I assume @gmmorris knows more ...

@gmmorris
Copy link
Contributor

gmmorris commented Nov 24, 2020 •
edited
Loading

LGTM, except that some "loadAlertState" tests have been deleted - is this function being tested elsewhere, so we're eliminating duplicate tests? If not, it seems like we shouldn't be deleting them, or at least we should create an issue to add them back (in some form) once this PR is merged. I assume @gmmorris knows more ...

We deleted the loadAlertState function itself (not just its tests) as it isn't in use in the Alerts plugin, only in the TriggersActionsPlugin, where it is duplicated along with its tests.

pmuellr
pmuellr approved these changes Nov 24, 2020
View reviewed changes
Copy link
Member

@pmuellr pmuellr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rudolf
Copy link
Contributor Author

rudolf commented Nov 26, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
alerts 27 22 -5
apm 1153 1152 -1
beatsManagement 302 301 -1
lists 128 125 -3
observability 111 110 -1
uptime 529 528 -1
total -12

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
apm 3.1MB 3.1MB +1.2KB
console 982.2KB 984.1KB +2.0KB
indexManagement 1.5MB 1.5MB +6.0KB
ingestPipelines 795.1KB 819.5KB +24.4KB
observability 169.1KB 170.4KB +1.2KB
searchprofiler 690.8KB 692.7KB +2.0KB
securitySolution 8.0MB 8.0MB +2.0B
triggersActionsUi 1.5MB 1.5MB +29.1KB
upgradeAssistant 147.4KB 154.1KB +6.7KB
uptime 983.8KB 985.0KB +1.2KB
total +73.8KB

Distributable file count

id before after diff
default 43013 43133 +120

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
alerts 89.4KB 66.4KB -23.0KB
beatsManagement 152.0KB 153.2KB +1.2KB
infra 174.3KB 181.0KB +6.7KB
lists 158.8KB 173.5KB +14.7KB
securitySolution 166.4KB 173.1KB +6.7KB
triggersActionsUi 161.0KB 161.2KB +270.0B
total +6.6KB

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@rudolf rudolf merged commit ee5c9bc into elastic:master Nov 26, 2020
@rudolf rudolf deleted the fp-ts-2.8.6 branch November 26, 2020 19:34
@rudolf rudolf mentioned this pull request Nov 26, 2020
Merged
rudolf added a commit that referenced this pull request Nov 26, 2020
@rudolf @gmmorris @kibanamachine
Upgrade fp-ts to 2.8.6 (#83866) (#84436)
aba0b31 
* Upgrade fp-ts to 2.8.6

* reduce import size from io-ts

* removed unused imports

* remove usage of fpts from alerts

Co-authored-by: Gidi Meir Morris <github@gidi.io>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Gidi Meir Morris <github@gidi.io>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 30, 2020
@gmmorris
Merge branch 'master' into alerts/custom-recovery-action-group
09732bd 
* master:
  [Security Solution] Exceptions Cypress tests (elastic#81759)
  [ML] Fix spaces job ID check (elastic#84404)
  [Security Solution][Detections] Handle dupes when processing threshold rules (elastic#83062)
  skip flaky suite (elastic#84440)
  skip flaky suite (elastic#84445)
  [APM] Fix missing `service.node.name` (elastic#84269)
  Upgrade fp-ts to 2.8.6 (elastic#83866)
  Added data streams privileges to better control delete actions in UI (elastic#83573)
  Improve short-url redirect validation (elastic#84366)
  TSVB offsets (elastic#83051)
  [Discover] Fix navigating back when changing index pattern (elastic#84061)
  [Logs UI] Polish the UI for the log entry examples in the anomaly table (elastic#82139)
  [Logs UI] Limit the height of the "view in context" container (elastic#83178)
  [Application Usage] Update `schema` with new `fleet` rename (elastic#84327)
  fix identation in list (elastic#84301)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmmorris gmmorris gmmorris left review comments

@mshustov mshustov mshustov left review comments

@pmuellr pmuellr pmuellr approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@rudolf @elasticmachine @gmmorris @kibanamachine @pmuellr @mshustov