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

Export a kibana restricted type definition #1239

Merged
merged 13 commits into from
Jul 6, 2020
Merged

Conversation

delvedor
Copy link
Member

@delvedor delvedor commented Jun 30, 2020

While working on #1243 and elastic/kibana#69905 we have decided that we should start shipping a restricted type definition file for Kibana, which hides the snake_cased and callbacks style APIs.

The type can be used as follows:

import { Client } from '@elastic/elasticsearch'
import { KibanaClient } from '@elastic/elasticsearch/api/kibana'

const client: KibanaClient = new Client({
  node: 'http://localhost:9200'
})

@delvedor delvedor mentioned this pull request Jul 2, 2020
15 tasks
@delvedor delvedor changed the title Added --kibana flag to client generator Export a kibana restricted type definition Jul 3, 2020
Copy link

@pgayvallet pgayvallet left a comment

Choose a reason for hiding this comment

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

This LGTM regarding our needs.

let oldTypeDefString = readFileSync(typeDefFile, 'utf8')
let start = oldTypeDefString.indexOf('/* GENERATED */')
let end = oldTypeDefString.indexOf('/* /GENERATED */')
let newTypeDefString = oldTypeDefString.slice(0, start + 15) + '\n' + types + '\n ' + oldTypeDefString.slice(end)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it's easy to forget to update length if we change start token. Do you think you could you refer it explicitly?

const startToken = '/* GENERATED */';
let startPosition = oldTypeDefString.indexOf(startToken)
let newTypeDefString = oldTypeDefString.slice(0, start + startToken.length) + ...

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't expect it to change, also, we could still forget to update it here :D

Copy link

Choose a reason for hiding this comment

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

@delvedor Jumping in on @restrry comment. The recommended change is not only to cover the case where "start token" would change, but also to avoid "magic" number as per our styleguide (https://github.com/elastic/kibana/blob/master/STYLEGUIDE.md#magic-numbersstrings).
When I read 15 it does not mean anything, when I read startToken.length it is explicit what it is. Just my 2 cents 😊

@delvedor delvedor merged commit 34a9176 into master Jul 6, 2020
@delvedor delvedor deleted the kibana-ts-output branch July 6, 2020 09:49
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.

4 participants