Skip to content

Tidy up serve.js - #158706

Closed
delanni wants to merge 3 commits into
elastic:mainfrom
delanni:kib-155137-serverjs-fixes
Closed

delanni wants to merge 3 commits into
elastic:mainfrom
delanni:kib-155137-serverjs-fixes

Conversation

@delanni

@delanni delanni commented May 31, 2023 •

Copy link
Copy Markdown
Member

Summary

Addresses #155137

Does some simple refactors as suggested in the original PR.

Checklist

@delanni delanni added release_note:skip Skip the PR/issue when compiling release notes v8.9.0 backport:skip This PR does not require backporting labels May 31, 2023
@delanni
delanni requested a review from a team as a code owner June 1, 2023 12:29
@kibana-ci

kibana-ci commented Jun 1, 2023 •

Copy link
Copy Markdown

💔 Build Failed

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #6 / bootstrap serverless should load additional serverless files for a valid project
  • [job] [logs] Jest Tests #6 / bootstrap serverless should skip loading the serverless files for an invalid project
  • [job] [logs] Jest Integration Tests #2 / Server configuration ordering adds dev configs to the queue
  • [job] [logs] Jest Integration Tests #2 / Server configuration ordering loads default config set without any options
  • [job] [logs] Jest Integration Tests #2 / Server configuration ordering loads serverless configs when --serverless is set
  • [job] [logs] Jest Integration Tests #2 / Server configuration ordering prefers --config options over default

Metrics [docs]

Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
securitySolution 401 405 +4
total +6

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
securitySolution 481 485 +4
total +6

History

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

@delanni delanni changed the title chore: Tidy up serve.js Tidy up serve.js Jun 1, 2023

@afharo afharo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

const root = new Root(rawConfigService, env, onRootShutdown);

const cliLogger = root.logger.get('cli');
cliLogger.info('Configurations parsed in this order: ' + env.configs.join(', '));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: since this log will be presented to users, should we explain that the ones on the right override the ones on the left?

@afharo afharo mentioned this pull request Jun 1, 2023
1 task done
[
'kibana.dev.yml',
'serverless.dev.yml',
// 'serverless.es.dev.yml' // Shouldn't this be loaded? It's mentioned in the README, but wasn't in the code

@jbudz jbudz Jun 1, 2023 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We've typically left the dev.yml convention uncommitted and untracked as a way for developers to persist setups. Optionally loaded.

@delanni

delanni commented Jun 2, 2023

Copy link
Copy Markdown
Member Author

Closing in favor of: #158750 and #158827

@delanni delanni closed this Jun 2, 2023
@delanni
delanni deleted the kib-155137-serverjs-fixes branch May 2, 2024 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v8.9.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants