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 eslint-config-kibana into core #12725

Merged
merged 38 commits into from
Jul 25, 2017

Conversation

kimjoar
Copy link
Contributor

@kimjoar kimjoar commented Jul 9, 2017

I mentioned this a while ago and people seemed open to the idea. Everything stays the same as before (e.g. same npm publish flow), it's just that we can now PR against this repo instead of having a separate repo that no one remembers to check.

/cc @epixa, @spalger

spalger and others added 30 commits February 9, 2016 16:12
[no-unused-vars]: Disallow declaration of variables that are not used in the code.
upgrade deps in preperation for babel6 transition
Disallow using 'context' in tests
* update deps

* include eslint-plugin-import
@kimjoar kimjoar added the review label Jul 9, 2017
@epixa
Copy link
Contributor

epixa commented Jul 10, 2017

I emphatically support moving our package dependencies into Kibana proper. Every time I encounter code we've written that is important for the development or function of Kibana that I need to go track down in a separate repo, I die a little inside.

@kimjoar kimjoar force-pushed the eslint-config-kibana-into-core branch from ebb2d95 to 4fbd3f2 Compare July 11, 2017 06:58
@kimjoar kimjoar removed the review label Jul 11, 2017
@cjcenizal cjcenizal added the Team:Operations Team label for Operations Team label Jul 12, 2017
@cjcenizal
Copy link
Contributor

@kjbekkelund Could you talk a little more about why we are still housing this as a "package" inside of a packages directory? What role will package.json play? Are we expected to bump its version number when we make changes to it?

Is there a problem with just storing the .eslint.js file in the root, and doing away with everything else?

@kimjoar
Copy link
Contributor Author

kimjoar commented Jul 12, 2017

@cjcenizal My goal here is purely pulling in external Kibana-related OSS repos that I feel should live in the main repo instead, so we can handle both issues and PRs in one place. I don't see the value in multiple repos for this stuff.

This PR is not exploring better setups for our eslint stuff, that feels like something we should do in a follow-up PR.

Re package.json, this should in my opinion be treated exactly as before. Make a change, up the version, npm publish, then upgrade version in both core and xpack. (at some later point when we upgrade to npm5 we could potentially explore something like npm install ./packages/eslint-config-kibana from the core, so we don't need to manually update versions (we would still have to do it for xpack)).

@kimjoar
Copy link
Contributor Author

kimjoar commented Jul 12, 2017

(I think one reason .eslint.js in the root of this repo is not enough is that we would have to copy/paste it into xpack)

@kimjoar
Copy link
Contributor Author

kimjoar commented Jul 12, 2017

There's a tiny bit of explanation in the readme in the packages folder, though not specific to eslint stuff, just the thinking around package.json etc: https://github.com/elastic/kibana/tree/master/packages

@cjcenizal
Copy link
Contributor

Ah, I forgot about X-Pack! Thanks for the explanation, this makes sense.

@kimjoar
Copy link
Contributor Author

kimjoar commented Jul 12, 2017

Btw, I was thinking about waiting with merging this until after ff, just so I'm 100% sure I don't break anything on something that's not important for 6.0. Feels like there's already so many things happening right now (and things breaking). Let me know if #12810 is essential in the short-term, then I can merge this asap instead of waiting.

@cjcenizal
Copy link
Contributor

@kjbekkelund I think we're OK. I was actually thinking the same thing... better to wait until after FF for this kind of stuff. :)

@kimjoar kimjoar merged commit bdaf848 into elastic:master Jul 25, 2017
@kimjoar kimjoar deleted the eslint-config-kibana-into-core branch July 25, 2017 08:02
cjcenizal pushed a commit to cjcenizal/kibana that referenced this pull request Jul 26, 2017
* Initial commit

* added actual config

* version 0.0.1

* version 0.0.2

* [no-const-assign] Disallow assignment to const

http://eslint.org/docs/rules/no-const-assign

* [no-redeclare] Disallow redeclaring variables

http://eslint.org/docs/rules/no-redeclare

* version 0.0.3

* [no-unused-vars]: Disallow declaration of variables that are not used in the code.

* Bump to 0.1.0.

* upgrade deps in preperation for babel6 transition

* 0.2.0-alpha1

* use yaml for readability

* 0.2.0

* update/pin peed dependency versions

* 0.2.1

* [quotes] allow template literals

This allows eslint to validate this rule from the styleguide: https://github.com/elastic/kibana/blob/master/style_guides/js_style_guide.md#use-template-strings-to-avoid-escaping-single-quotes

* 0.2.2

* add object-curly-spacing and no-global-assign rules

* sort .eslintrc.yaml rules

* 0.3.0

* add basic react support

* 0.4.0

* Disallow using 'context' in tests

* 0.5.0

* move from .eslintrc.yaml to .eslintrc.js without .json generation (#6)

* Implement import plugin (#7)

* update deps

* include eslint-plugin-import

* Dereference import config (#8)

* reorganize existing rules into groups

* defreference eslint-plugin-import "recommended" config

Based on https://github.com/benmosher/eslint-plugin-import/blob/ea9c92c7324473ef303ac76b127e17af2becd2ee/config/recommended.js

* 0.6.0

* set environment info for import rule

* 0.6.1

* update peerDependencies

* 0.7.0

* Move eslint-config-kibana into packages directory
cjcenizal added a commit that referenced this pull request Jul 26, 2017
* Initial commit

* added actual config

* version 0.0.1

* version 0.0.2

* [no-const-assign] Disallow assignment to const

http://eslint.org/docs/rules/no-const-assign

* [no-redeclare] Disallow redeclaring variables

http://eslint.org/docs/rules/no-redeclare

* version 0.0.3

* [no-unused-vars]: Disallow declaration of variables that are not used in the code.

* Bump to 0.1.0.

* upgrade deps in preperation for babel6 transition

* 0.2.0-alpha1

* use yaml for readability

* 0.2.0

* update/pin peed dependency versions

* 0.2.1

* [quotes] allow template literals

This allows eslint to validate this rule from the styleguide: https://github.com/elastic/kibana/blob/master/style_guides/js_style_guide.md#use-template-strings-to-avoid-escaping-single-quotes

* 0.2.2

* add object-curly-spacing and no-global-assign rules

* sort .eslintrc.yaml rules

* 0.3.0

* add basic react support

* 0.4.0

* Disallow using 'context' in tests

* 0.5.0

* move from .eslintrc.yaml to .eslintrc.js without .json generation (#6)

* Implement import plugin (#7)

* update deps

* include eslint-plugin-import

* Dereference import config (#8)

* reorganize existing rules into groups

* defreference eslint-plugin-import "recommended" config

Based on https://github.com/benmosher/eslint-plugin-import/blob/ea9c92c7324473ef303ac76b127e17af2becd2ee/config/recommended.js

* 0.6.0

* set environment info for import rule

* 0.6.1

* update peerDependencies

* 0.7.0

* Move eslint-config-kibana into packages directory
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Operations Team label for Operations Team v6.0.0-rc1 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants