-
Notifications
You must be signed in to change notification settings - Fork 725
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
Conversation
There was a problem hiding this 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) |
There was a problem hiding this comment.
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) + ...
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 😊
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: