Skip to content
This repository has been archived by the owner on Nov 19, 2017. It is now read-only.

Add jest plugin and settings #9

Closed
wants to merge 1 commit into from

Conversation

weltenwort
Copy link
Member

This extracts the ui-specific eslint settings from elastic/kibana so they can
be applied to other repositories as well.

@weltenwort weltenwort self-assigned this Jul 19, 2017
weltenwort added a commit to weltenwort/kibana that referenced this pull request Jul 19, 2017
The jest-related eslint configuration is located in the shared configuration
once elastic/eslint-config-kibana#9 has been merged.
@kimjoar
Copy link
Contributor

kimjoar commented Jul 20, 2017

When this is merged, can you push a new version and create PRs for core and xpack?

@spalger
Copy link

spalger commented Jul 20, 2017

I use the kibana presets in non-kibana projects that I really don't want to depend on jest. Can we add this as a supplementary config that needs opt-in via extends?

if it exposed a jest.js file at the root of the package this should work:

---
extends:
  - "@elastic/kibana"
  - "@elastic/kibana/jest"

@kimjoar
Copy link
Contributor

kimjoar commented Jul 20, 2017

-1 from me. imo this is the Kibana eslint config, and should be optimized for those projects (It's not a big deal, but we shouldn't have to think about this when adding other plugins later either imo)

@kimjoar
Copy link
Contributor

kimjoar commented Jul 20, 2017

(Though, maybe it makes sense if there are other elastic projects that rely on them but not jest? Open to the idea)

@spalger
Copy link

spalger commented Jul 20, 2017

(Though, maybe it makes sense if there are other elastic projects that rely on them but not jest? Open to the idea)

That's what I'm describing, other elastic js projects that reuse the kibana config but aren't using jest. We can improve that after this is merged though, LGTM

@kimjoar
Copy link
Contributor

kimjoar commented Jul 20, 2017

Ah, I misunderstood the "non-kibana projects" part. Works for me.

This extracts the ui-specific eslint settings from elastic/kibana so they can
be applied to other repositories as well.
@weltenwort
Copy link
Member Author

@spalger makes sense, thanks for the suggestion. I updated the PR.

@weltenwort
Copy link
Member Author

When adding some documentation I noticed that we are referencing the config as @elastic/kibana in Kibana's root .eslintrc even though the documentation states:

The module name can also be customized, just note that when using scoped modules it is not possible to omit the eslint-config- prefix. Doing so would result in package naming conflicts, and thus in resolution errors in most of cases.

Is that something we want to change as well to align with the official docs?

@spalger
Copy link

spalger commented Jul 21, 2017

@weltenwort if it works in eslint 3 then I think we should, but pretty sure that was a breaking change added in eslint 4

@weltenwort
Copy link
Member Author

I think specifying the fully qualified name was always supported. There was a breaking change in 4 that disallows shorthands in namespaced packages to fix misbehavior resulting from ambiguities. I will change it in the PR that references this in kibana after this is merged and published.

@spalger any other concerns?

@weltenwort
Copy link
Member Author

replaced by elastic/kibana#13090

@weltenwort weltenwort closed this Jul 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants