PR Review Updates #6
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ver concerns from frontend code, and start enforcing snake case in simulate api
BigFunger
pushed a commit
that referenced
this pull request
Sep 6, 2016
BigFunger
pushed a commit
that referenced
this pull request
Aug 3, 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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@BigFunger I've made some updates to your first PR that I'd like you to review and, if you're good with them, merge into your branch before we merge the PR into the Kibana feature/ingest branch.
Here are the highlights of what I changed:
getDefinitionmethods out of ingest_processor_types.js and into their own special "converters" module. I'm guessing this might be the most controversial change, so let me explain my reasoning. I've found in the past that it's best to separate model objects (I use that term a bit loosely here) from the logic that serializes them to different formats. It helps keep the ES specific knowledge encapsulated on the server side and it decouples our models from a specific serialization format which gives us the flexibility to provide different types of converters in the future (for instance, one for the "Kibana processor schema" and one for the "ES processor schema"). It also establishes a nice pattern for converting any Kibana API document to an ES API document in the future, all the converters can live in the same place so they're easy to find and manage.callWithRequestinstead of the raw elasticsearch.js client so that authorization headers are passed along.Let me know what you think.