Move @elastic/datemath into monorepo #16531

kimjoar · 2018-02-05T19:38:32Z

This pulls in @elastic/datemath to the monorepo, renames it to @kbn/datemath, and cleans up a couple things.

This will require yarn kbn bootstrap instead of just yarn, as you'll otherwise see errors like:

/Users/kim/dev/elastic/master/kibana/src/core_plugins/kibana/public/discover/controllers/discover.js
  6:22  error  Unable to resolve path to module '@kbn/datemath'  import/no-unresolved

or

ERROR in ./src/ui/public/vislib/visualizations/time_marker.js
Module not found: Error: Can't resolve '@kbn/datemath' in '/Users/kim/dev/elastic/master/kibana/src/ui/public/vislib/visualizations'

as the @kbn/datemath package needs to be built before it can be used.

NOTE: This does not work with Kibana plugins until #16509 and elastic/kibana-plugin-helpers#57 are merged.

https://github.com/elastic/x-pack-kibana/pull/4505 is using it in x-pack.

epixa · 2018-02-05T19:41:00Z

This is a package, not a plugin, right?

kimjoar · 2018-02-05T19:45:12Z

🙈 Yep. Still a WIP, I'm just exploring the flow when using a package like this across Kibana and x-pack (or any other plugin).

elasticmachine · 2018-02-06T01:53:29Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

kimjoar · 2018-02-06T09:59:38Z

Okey, this is ready to review

azasypkin

Code LGTM, just a couple of minor nits/questions + please add trailing new lines to all new files. Going to run Kibana and find the places where this package is used, so keeping r? for now

azasypkin · 2018-02-06T14:47:59Z

packages/kbn-build/README.md

@@ -117,6 +117,9 @@ For more details, run:
 yarn kbn
 ```

+Bootstrapping also calls the `kbn:boostrap` script for every included project.
+This is intended for packages that needs to be built/transpiled to be usable.


typo: that needs ---> that need?

azasypkin · 2018-02-06T15:02:42Z

packages/kbn-build/src/commands/bootstrap.js

+
+  /**
+   * At the end of the bootstrapping process we call all `kbn:bootstrap` scripts
+   * in the list of projects. We do this because some projects needs to be


typo: needs --> need.

azasypkin · 2018-02-06T16:48:23Z

packages/kbn-datemath/.gitignore

+lib
+node_modules
+npm-debug.log
+yarn.lock


question: don't we want to commit yarn.lock? Also do we need .gitignore in the package itself?

Same question for .npmignore, why do we need this? We have private: true in the package.json so it doesn't seem like we want to ever publish this package.

yeah, yarn.lock can stay now that we aren't installing from npm, and the rest of the stuff in this file is redundant, so I removed the file

Also the .npmignore file

++ good catch

azasypkin · 2018-02-06T17:31:57Z

src/cli_plugin/install/project_dependencies.js

+export async function prepareProjectDependencies(settings, logger) {
+  await handleLinkDependencies(
+    settings.workingPath,
+    logger


nit: looks like you don't use logger in handleLinkDependencies.

azasypkin · 2018-02-06T17:43:53Z

src/cli_plugin/install/project_dependencies.js

+    const depVersion = deps[depName];
+
+    if (depVersion.startsWith(LINK_DEP)) {
+      if (isKibanaDep) {


issue: it's always true since you don't call isKibanaDep function at all (looks like it needs a simple test) :)

Also is there any reason why we don't want to just use if (depVersion.startsWith(LINK_DEP) && !isKibanaDep(depVersion) without nested empty if? We can easily expand it once we need it.

Haha 🙈 I've added some tests for it. And fixed the if too (initially did it like that because I had code in the "kibana-if" too, but yeah, this is cleaner)

elasticmachine · 2018-02-07T00:58:53Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

kimjoar · 2018-02-07T10:21:56Z

Btw, here's a tiny Kibana plugin I created using the plugin-helpers changes (elastic/kibana-plugin-helpers#57) that relies on @kbn/datemath:
my-kbn-plugin-0.0.0.zip

First build Kibana (yarn build --skip-archives --skip-os-packages), then cd into build/kibana-... for your architecture, then run:

./bin/kibana-plugin install file:./path/to/my-kbn-plugin-0.0.0.zip

and you should see something like this in the output when starting the Kibana server:

{ units: [ 'y', 'M', 'w', 'd', 'h', 'm', 's', 'ms' ] }

(you'll see it during optimize after plugin install, as optimize starts the kibana server)

EDIT: updated zip, should now log at startup on server, when the api endpoint is hit, and when opening the plugin in the ui.

azasypkin · 2018-02-07T10:23:35Z

Btw, here's a tiny Kibana plugin I created using the plugin-helpers changes that relies on @kbn/datemath: my-kbn-plugin-0.0.0.zip

Ah, nice! I was about to ask you how to test install part :)

elasticmachine · 2018-02-07T10:32:32Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-02-07T12:07:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

azasypkin

Haven't noticed any issues while running Kibana and installing of the example plugin, LGTM

kimjoar · 2018-02-07T12:34:51Z

tasks/jenkins.js

-    // e.g. imports resolve properly. If we don't build the packages here, the
-    // `main` field in their `package.json` would link to a location that
-    // doesn't exist yet.
-    'buildPackages',


This is no longer needed because of the change to kbn bootstrap (it runs kbn:bootstrap scripts defined in package.json in all packages)

spalger

LGTM

elasticmachine · 2018-02-07T18:05:21Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-02-08T14:13:12Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-02-08T15:11:27Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

spalger · 2018-02-09T23:00:09Z

packages/kbn-datemath/.babelrc

+{
+  "presets": [["env", {
+    "targets": {
+      "node": "6.0",


We should use node: current rather than pinning to node 6

spalger · 2018-02-09T23:00:19Z

packages/kbn-datemath/package.json

+  },
+  "devDependencies": {
+    "babel-cli": "^6.26.0",
+    "babel-core": "^6.26.0",


babel-core isn't necessary

elasticmachine · 2018-02-12T11:46:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

epixa · 2018-02-12T18:29:07Z

packages/kbn-datemath/package.json

@@ -0,0 +1,20 @@
+{
+  "name": "@kbn/datemath",
+  "version": "5.0.0",


We shouldn't need to version this. We should only even need to include a license if it's forked from a separate project ~~(which this particular package is)~~.

Haha, I'm sooo used to npm complaining that I've added because of that. But yarn isn't complaining about missing version, so I'll 🔥 it

Npm wouldn't complain either because this package is marked as private, which disables pretty much all of the npm integrity checks.

Aaand it's a no-go:

18:35:14 Installing dependencies in [kibana]: 18:35:14 18:35:15 $ node ./preinstall_check 18:35:15 [1/5] Validating package.json... 18:35:15 [2/5] Resolving packages... 18:35:15 error Package "@kbn/datemath@" doesn't have a "version". 18:35:15 info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

What? Why on earth would it do that? Maintaining versions on these packages doesn't make any sense, and it's definitely going to get out of date really fast. I'd argue it's out of date the moment it is contributed to the project :P

Why? I expect because link: and private: true are edge-cases.

Just curious, and this seems like a decent place to ask; why not just pin all the package versions to the Kibana version in the root, ala lerna's default behavior (ie. not independent mode)? They're all technically new packages, so it's not like we'd be rolling back versions or anything. And they're all pretty hard-bound to Kibana anyway... and we're also not publishing any of them.

++ I've been thinking about the same thing. We could write some tooling in yarn kbn to handle it. It could require exact version match on all private: true packages, for example. That way we could still have publishable packages, if we want to (not sure we do want that any more, though, so it might not be necessary at all and we just require exact same version on every package)

Yeah, that's what I was thinking too. A kbn version, similar to npm version (or I assume lerna, I've only used lerna with independent versioning so far) that would just pump core and all the packages with it.

elasticmachine · 2018-02-12T19:19:55Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

This reverts commit 8924a0b.

elasticmachine · 2018-02-12T20:08:28Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-02-12T23:26:11Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-02-13T20:48:45Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

elasticmachine · 2018-02-13T21:37:07Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

kimjoar added the Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc label Feb 5, 2018

kimjoar changed the title ~~Production plugin example: datemath~~ Production package example: datemath Feb 5, 2018

kimjoar added the WIP Work in progress label Feb 5, 2018

kimjoar force-pushed the platform/import-datemath branch 3 times, most recently from adc06f5 to 4baff7e Compare February 5, 2018 23:43

elastic deleted a comment from elasticmachine Feb 6, 2018

kimjoar removed the WIP Work in progress label Feb 6, 2018

azasypkin self-requested a review February 6, 2018 14:35

azasypkin reviewed Feb 6, 2018

View reviewed changes

spalger mentioned this pull request Feb 7, 2018

Move kibana-plugin-helpers to kbn-plugin-helpers package #16565

Merged

azasypkin approved these changes Feb 7, 2018

View reviewed changes

kimjoar commented Feb 7, 2018

View reviewed changes

spalger approved these changes Feb 7, 2018

View reviewed changes

kimjoar mentioned this pull request Feb 7, 2018

Run kbn:bootstrap script after installing deps #16585

Merged

kimjoar force-pushed the platform/import-datemath branch 2 times, most recently from 284ffe9 to 368664b Compare February 8, 2018 14:11

check in the code

08fedde

kimjoar and others added 3 commits February 8, 2018 15:19

Fix missing sinon clock restore

09ed19b

[yarn] remove yarn.lock from .gitiginore

1fb4658

[kbn/datemath] remove pointless git and npm ignore files

3a92092

kimjoar force-pushed the platform/import-datemath branch from f9ced51 to 3a92092 Compare February 8, 2018 14:19

spalger reviewed Feb 9, 2018

View reviewed changes

kimjoar added 2 commits February 12, 2018 11:53

Babel cleanups

ea661ac

Merge branch 'master' into platform/import-datemath

53c6648

Merge branch 'master' into platform/import-datemath

e26edb5

epixa reviewed Feb 12, 2018

View reviewed changes

Remove version and license fields from package.json

8924a0b

Revert "Remove version and license fields from package.json"

9d6f34f

This reverts commit 8924a0b.

Merge branch 'master' into platform/import-datemath

5732cbb

kimjoar added 2 commits February 13, 2018 16:57

Merge branch 'master' into platform/import-datemath

8c4919d

Merge branch 'master' into platform/import-datemath

7e33ffc

kimjoar merged commit 1c3b404 into elastic:master Feb 13, 2018

kimjoar changed the title ~~Production package example: datemath~~ Move @elastic/datemath into monorepo Feb 13, 2018

kimjoar mentioned this pull request Feb 13, 2018

[6.x] Production package example: datemath (#16531) #16719

Merged

kimjoar added a commit to kimjoar/kibana that referenced this pull request Feb 13, 2018

Production package example: datemath (elastic#16531)

ecffc23

kimjoar added a commit that referenced this pull request Feb 13, 2018

Production package example: datemath (#16531) (#16719)

3529667

kimjoar added v6.3.0 v7.0.0 labels Feb 21, 2018

kimjoar deleted the platform/import-datemath branch April 18, 2018 10:02

Move @elastic/datemath into monorepo #16531

Move @elastic/datemath into monorepo #16531

Conversation

kimjoar commented Feb 5, 2018 • edited Loading

epixa commented Feb 5, 2018

kimjoar commented Feb 5, 2018

elasticmachine commented Feb 6, 2018

💚 Build Succeeded

kimjoar commented Feb 6, 2018 • edited Loading

azasypkin left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Feb 7, 2018

💚 Build Succeeded

kimjoar commented Feb 7, 2018 • edited Loading

azasypkin commented Feb 7, 2018

elasticmachine commented Feb 7, 2018

💚 Build Succeeded

elasticmachine commented Feb 7, 2018

💚 Build Succeeded

azasypkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spalger left a comment

Choose a reason for hiding this comment

elasticmachine commented Feb 7, 2018

💚 Build Succeeded

elasticmachine commented Feb 8, 2018

💚 Build Succeeded

elasticmachine commented Feb 8, 2018

💚 Build Succeeded

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Feb 12, 2018

💚 Build Succeeded

epixa Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

kimjoar Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

w33ble Feb 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Feb 12, 2018

💔 Build Failed

elasticmachine commented Feb 12, 2018

💚 Build Succeeded

elasticmachine commented Feb 12, 2018

💚 Build Succeeded

elasticmachine commented Feb 13, 2018

💚 Build Succeeded

elasticmachine commented Feb 13, 2018

💚 Build Succeeded

kimjoar commented Feb 5, 2018 •

edited

Loading

kimjoar commented Feb 6, 2018 •

edited

Loading

azasypkin left a comment •

edited

Loading

kimjoar commented Feb 7, 2018 •

edited

Loading

epixa Feb 12, 2018 •

edited

Loading

kimjoar Feb 12, 2018 •

edited

Loading

w33ble Feb 13, 2018 •

edited

Loading