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

Default to named ui/public: a to m #11200

Merged

Conversation

stacey-gammon
Copy link
Contributor

Related to #10981. Converting rest of ui/public to using named exports only. Default exports may still be around where they are used prolifically, or in x-pack.

Background: #8641

@stacey-gammon stacey-gammon changed the title Default to named ui public a to m Default to named ui/public: a to m Apr 12, 2017
@stacey-gammon stacey-gammon force-pushed the default-to-named-ui-public-a-to-m branch 2 times, most recently from b72f688 to 6512541 Compare April 12, 2017 20:14
@stacey-gammon stacey-gammon force-pushed the default-to-named-ui-public-a-to-m branch 2 times, most recently from f8e1322 to 178cc23 Compare April 13, 2017 13:55
@stacey-gammon stacey-gammon requested a review from BigFunger April 13, 2017 15:32
@BigFunger
Copy link
Contributor

@stacey-gammon, Please resolve the conflicts

@stacey-gammon stacey-gammon force-pushed the default-to-named-ui-public-a-to-m branch from 178cc23 to 0631836 Compare April 13, 2017 15:59
@stacey-gammon
Copy link
Contributor Author

@BigFunger, done!

@stacey-gammon stacey-gammon requested review from lukasolson and removed request for BigFunger April 13, 2017 16:26
@stacey-gammon
Copy link
Contributor Author

stacey-gammon commented Apr 13, 2017

@lukasolson You are the winning choice of reviewer (you can thank @BigFunger 😸 ).

If you can't get to it today or tomorrow, lmk and I'll continue my search.

P.S. as reviewer, since the changes are really too great to peruse over individually, I think it's sufficient to check it out locally, explore the product a bit, and keep an eye out in the console for 'module undefined' errors.

@stacey-gammon stacey-gammon force-pushed the default-to-named-ui-public-a-to-m branch from 0631836 to c560141 Compare April 13, 2017 20:32
Copy link
Member

@lukasolson lukasolson left a comment

Choose a reason for hiding this comment

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

LGTM, left a couple comments inline.

import getPercentileValue from './percentiles_get_value';
import { AggTypesMetricsMetricAggTypeProvider } from 'ui/agg_types/metrics/metric_agg_type';
import { AggTypesMetricsGetResponseAggConfigClassProvider } from 'ui/agg_types/metrics/get_response_agg_config_class';
import{ getPercentileValue } from './percentiles_get_value';
Copy link
Member

Choose a reason for hiding this comment

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

Not a big deal, but missing a space here.

@@ -1,7 +1,7 @@
import sinon from 'auto-release-sinon';
import expect from 'expect.js';
import ngMock from 'ng_mock';
import FilterBarLibGenerateMappingChainProvider from 'ui/filter_bar/lib/generate_mapping_chain';
import { GenerateMappingChainProvider } from 'ui/filter_bar/lib/generate_mapping_chain';
Copy link
Member

Choose a reason for hiding this comment

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

Just curious, why remove the FilterBarLib prefix here but not below in FilterBarLibMapAndFlattenFiltersProvider?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, not sure why I did it that way, but does look like I kept the prefixes on all the others, so will switch back to keeping the prefix here!

@stacey-gammon stacey-gammon force-pushed the default-to-named-ui-public-a-to-m branch from c560141 to 8a6663b Compare April 14, 2017 19:27
@stacey-gammon stacey-gammon merged commit 9f33293 into elastic:master Apr 16, 2017
stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Apr 16, 2017
* Convert remaining export default's in ui/public

* Chunk 2

* Part 3

* Part 4

* Part 5

Note: Must check in https://github.com/elastic/x-pack-kibana/pull/938
first

* Part 6

* Part 6

* Part 8

* Part 9

* address code review comments
stacey-gammon added a commit that referenced this pull request Apr 17, 2017
* Convert remaining export default's in ui/public

* Chunk 2

* Part 3

* Part 4

* Part 5

Note: Must check in https://github.com/elastic/x-pack-kibana/pull/938
first

* Part 6

* Part 6

* Part 8

* Part 9

* address code review comments
@stacey-gammon stacey-gammon deleted the default-to-named-ui-public-a-to-m branch October 24, 2017 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants