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.

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

Move more directories into src/legacy #30435

Merged
merged 6 commits into from
Feb 11, 2019
Merged

Move more directories into src/legacy #30435

merged 6 commits into from
Feb 11, 2019

Conversation

epixa
Copy link
Contributor

@epixa epixa commented Feb 7, 2019

There are six commits here, and they will be merged as-is rather than squashed first.

The first five commits perform the following moves:

src/deprecation      -> src/legacy/deprecation
src/ui               -> src/legacy/ui
src/server           -> src/legacy/server
src/plugin_discovery -> src/legacy/plugin_discovery
src/utils            -> src/legacy/utils

The final commit cleans up path references across the project into and out of those locations.

This is to support the new platform migration effort, and the target structure as a whole is described in #12466.

Note, I might backport this to 6.7.0. It depends how seamless that turns out to be.

@epixa epixa self-assigned this Feb 7, 2019
@epixa epixa requested review from a team as code owners February 7, 2019 21:40
@elasticmachine

This comment has been minimized.

@epixa epixa requested a review from a team as a code owner February 8, 2019 01:20
@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@epixa epixa added Feature:Plugins Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc non-issue Indicates to automation that a pull request should not appear in the release notes labels Feb 8, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@epixa epixa added the review label Feb 10, 2019
@epixa
Copy link
Contributor Author

epixa commented Feb 10, 2019

@spalger This is ready for review when you get a chance tomorrow.

@@ -91,8 +91,7 @@ export class Build {
sourceMap: true,
sourceMapEmbed: true,
includePaths: [
resolve(__dirname, '../..'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is really the change in this PR that isn't strictly just adjusting paths. The existing sass implementation for plugins allowed importing from anywhere inside src. I spoke to @tylersmalley about this and we agreed to just let plugin sass import relative to root rather than update this to import from within src/legacy.

There were a couple reasons for this:

  1. We can't statically analyze sass, so any sort of bulk updates to paths will need to be done through find/replace. Something more specific like src/legacy/ui is much easier to grep for than just ui.
  2. If you don't know exactly where to look, finding this path configuration is challenging since it's just string-based relative paths.
  3. Why not treat paths a little less magically?

We both also agreed that node_modules should probably be removed as well so folks would need to import those relative root as well, but that felt out of scope of this PR since it wasn't already touching any of those imports.

Copy link
Contributor

@mikecote mikecote left a comment

Choose a reason for hiding this comment

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

Changes LGTM, pulled locally and everything ran fine 👍

@epixa epixa merged commit 4aa2a4c into elastic:master Feb 11, 2019
@epixa epixa deleted the new-dir branch February 11, 2019 15:52
epixa added a commit that referenced this pull request Feb 11, 2019
Moves the following directories into src/legacy:

src/deprecation         -> src/legacy/deprecation
src/ui                         -> src/legacy/ui
src/server                  -> src/legacy/server
src/plugin_discovery -> src/legacy/plugin_discovery
src/utils                     -> src/legacy/utils
epixa added a commit that referenced this pull request Feb 11, 2019
Moves the following directories into src/legacy:

src/deprecation         -> src/legacy/deprecation
src/ui                         -> src/legacy/ui
src/server                  -> src/legacy/server
src/plugin_discovery -> src/legacy/plugin_discovery
src/utils                     -> src/legacy/utils
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev Feature:Plugins non-issue Indicates to automation that a pull request should not appear in the release notes review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants