-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
chore: cleanup as unknown conversion #19587
Conversation
@@ -285,7 +285,8 @@ describe('buildQueryObject', () => { | |||
datasource: '5__table', | |||
granularity_sqla: 'ds', | |||
viz_type: 'table', | |||
url_params: null as unknown as undefined, | |||
// @ts-ignore |
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.
ignore is used for the test case.
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 believe // @ts-expect-error
is the correct syntax for testing with incorrect types
@@ -226,7 +226,7 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) { | |||
value: xt, | |||
label: xt, | |||
}))} | |||
value={from as unknown as undefined} | |||
value={{ value: from, label: from }} |
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.
bycatch: use object fill in Select value
@@ -235,7 +235,7 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) { | |||
name="select-to" | |||
placeholder={t('[To]-')} | |||
options={TIME_OPTIONS.map(xt => ({ value: xt, label: xt }))} | |||
value={to as unknown as undefined} | |||
value={{ value: to, label: to }} |
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.
same before
@@ -247,7 +247,7 @@ function QuerySearch({ actions, displayLimit }: QuerySearchProps) { | |||
value: s, | |||
label: s, | |||
}))} | |||
value={status as unknown as undefined} | |||
value={{ value: status, label: status }} |
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.
same before
filterSets: {}, | ||
filters: { |
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.
useless fields
const engineParamsCatalog = Object.keys( | ||
const engineParamsCatalog = Object.entries( | ||
extra_json?.engine_params?.catalog || {}, | ||
).map(e => ({ | ||
name: e, | ||
value: extra_json?.engine_params?.catalog[e], | ||
).map(([key, value]) => ({ | ||
name: key, | ||
value, |
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.
same logical substitution
catalog: Record<any, any> | string; | ||
catalog?: Record<any, any> | string; |
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.
catalog
is an optional field in engine params
f78a03b
to
001b975
Compare
Codecov Report
@@ Coverage Diff @@
## master #19587 +/- ##
==========================================
+ Coverage 66.65% 66.68% +0.02%
==========================================
Files 1680 1681 +1
Lines 64292 64309 +17
Branches 6564 6565 +1
==========================================
+ Hits 42855 42885 +30
+ Misses 19735 19731 -4
+ Partials 1702 1693 -9
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
export function addAlpha(color: string, opacity: number): string { | ||
// coerce values so ti is between 0 and 1. | ||
const rounded = Math.round(Math.min(Math.max(opacity || 1, 0), 1) * 255); | ||
return color + rounded.toString(16).toUpperCase(); | ||
// opacity value should be between 0 and 1. | ||
if (opacity > 1 || opacity < 0) { | ||
throw new Error( | ||
`The alpha channel should between 0 and 1, but got: ${opacity}`, | ||
); | ||
} | ||
// the alpha value is between 00 - FF | ||
const alpha = `0${Math.round(opacity * 255) | ||
.toString(16) | ||
.toUpperCase()}`.slice(-2); | ||
|
||
return `${color}${alpha}`; |
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.
bycatch: fix codes coverage.
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.
Thanks for making the code cleaner! One nit, other than that LGTM
@@ -285,7 +285,8 @@ describe('buildQueryObject', () => { | |||
datasource: '5__table', | |||
granularity_sqla: 'ds', | |||
viz_type: 'table', | |||
url_params: null as unknown as undefined, | |||
// @ts-ignore |
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 believe // @ts-expect-error
is the correct syntax for testing with incorrect types
SUMMARY
Clean up as unknown after monorepo
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
TESTING INSTRUCTIONS
Pass all CI
ADDITIONAL INFORMATION