Env contains distributable flag #40361
Conversation
mshustov
added
review
Team:Core
|
Pinging @elastic/kibana-platform |
💔 Build Failed |
mshustov
force-pushed
the
issue-40187-is-dist-env
branch
from
b3ea169
to
07f1eaf
Compare
| @@ -116,6 +118,7 @@ Env { | |||
| "branch": "some-branch", | |||
| "buildNum": 9007199254740991, | |||
| "buildSha": "XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX", | |||
| "dist": undefined, | |||
There was a problem hiding this comment.
note: hmm, can we make sure isKibanaDistributable and dist are always boolean? Just for the sake of consistency.
There was a problem hiding this comment.
agree, TS fooled me
💔 Build Failed |
| @@ -138,6 +139,7 @@ export class Env { | |||
| buildNum: isKibanaDistributable ? pkg.build.number : Number.MAX_SAFE_INTEGER, | |||
| buildSha: isKibanaDistributable ? pkg.build.sha : 'XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX', | |||
| version: pkg.version, | |||
| dist: isKibanaDistributable, | |||
There was a problem hiding this comment.
Although we used dist in LP what do you think of using a more descriptive key, we could use the full word distributable but isKibanaDistributable is probably even more descriptive like:
this.packageInfo = Object.freeze({
...
version: pkg.version,
isKibanaDistributable,
...
});There was a problem hiding this comment.
I wish we did, but suspect it slows down migration as require manual changing. I'd rename this flag when plugins migrated to NP and implemented with Typescript.
There was a problem hiding this comment.
OK, how and where we introduce breaking changes probably warrants a bigger discussion and doesn't need to block this pr.
💚 Build Succeeded |
* env contains distributable flag * make sure flag is always boolean
mshustov
moved this from Needs Backport
to Done (this week)
in kibana-core [DEPRECATED]
* env contains distributable flag * make sure flag is always boolean
Summary
Close #40187
This information is available in for Legacy config https://github.com/elastic/kibana/blob/master/src/legacy/server/config/config.js#L113 and expected to be present for config schema validation in New platform.
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 IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers