-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Partial Response : "Use Partial Response" enabled by default #6977
Partial Response : "Use Partial Response" enabled by default #6977
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.
Thanks! Generally looks good, but I wonder if you came across the issue mentioned in comment: #6270 (comment)
@@ -272,6 +276,7 @@ const PanelList: FC<RouteComponentProps & PathPrefixProps> = ({ pathPrefix = '' | |||
defaultStep={defaultStep} | |||
defaultEngine={defaultEngine} | |||
queryHistoryEnabled={enableQueryHistory} | |||
usePartialResponse={usePartialResponse === 'true'} |
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.
Do we need the equality check here?
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.
Yaa actually i used it to resolve a error which ensure it's always a boolean.
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.
Isn't this done in the js/ts world by doing a double negation, like !!usePartialResponse
? I see it often where people want to convert something to boolean.
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.
Thank you @douglascamata made changes suggested by you its working ! can review now
Hello @saswatamcode worked on the comment you suggested tested it on my local machine you can review it again ! |
5a7256f
to
0cc822b
Compare
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.
lgtm
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. LGTM.
I am not expert on the UI part but the code looks ok.
@Vanshikav123 Please sign off DCO |
Rebuild Signed-off-by: Vanshika <[email protected]>
9c9b6da
to
e77caa8
Compare
Done ! |
Changes
I have made relevant changes in the codebase such that #6270 issue is resolved and Use Partial Response checkbox is enabled by default.