-
Notifications
You must be signed in to change notification settings - Fork 20
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
Support internal kind #1148
Support internal kind #1148
Conversation
/deploy |
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.
Reviewed 17 of 19 files at r1, 3 of 3 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @reggev)
core/datasources-service/bootstrap.js, line 61 at r1 (raw file):
process.on('beforeExit'
exit is never called...
core/datasources-service/bootstrap.js, line 59 at r2 (raw file):
} async _handleExit() {
unused
/deploy |
/deploy dev1 |
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.
Reviewable status: 2 of 26 files reviewed, 2 unresolved discussions (waiting on @yehiyam)
core/datasources-service/bootstrap.js, line 61 at r1 (raw file):
Previously, yehiyam wrote…
process.on('beforeExit'
exit is never called...
Done.
core/datasources-service/bootstrap.js, line 59 at r2 (raw file):
Previously, yehiyam wrote…
unused
Done.
/deploy dev1 |
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.
Reviewed 13 of 18 files at r3, 11 of 11 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @reggev)
core/datasources-service/package.json, line 20 at r4 (raw file):
"@hkube/storage-manager": "^2.0.33", "@hkube/uid": "^1.0.4", "@homer0/prettier-plugin-jsdoc": "^2.0.0",
why not in dev-dependencies
BTW, this package has 200 downloads and 3 stars. I don't think we should use it
/deploy |
/deploy |
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.
Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @reggev)
/deploy |
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.
Reviewed 6 of 6 files at r7.
Reviewable status: complete! all files reviewed, all discussions resolved
* improved s3 validation * updated docs and validation * updated dependencies, fixed lint issues * updated structuring url on init token * added bypass on removeStoredToken * fixed invalid token handing on internal clone * updated dependencies and prettierrc * cleanups * fixed repository_url * fixed port set to NaN issue * dropped useSSL property from create datasource * change docker CMD to allow remote debugging Co-authored-by: Yehiyam Livneh <yehiyam@gmail.com> Co-authored-by: yehiyam <yehiyam@users.noreply.github.com> .... bump version [skip ci]
* improved s3 validation * updated docs and validation * updated dependencies, fixed lint issues * updated structuring url on init token * added bypass on removeStoredToken * fixed invalid token handing on internal clone * updated dependencies and prettierrc * cleanups * fixed repository_url * fixed port set to NaN issue * dropped useSSL property from create datasource * change docker CMD to allow remote debugging Co-authored-by: Yehiyam Livneh <yehiyam@gmail.com> Co-authored-by: yehiyam <yehiyam@users.noreply.github.com> .... bump version [skip ci]
This change is