Skip to content
This repository has been archived by the owner on May 10, 2019. It is now read-only.

fix #66 random text response #67

Closed
wants to merge 2 commits into from

Conversation

abhisharma404
Copy link

The random text query bug is fixed, this is the result :

fixed

@abhisharma404 abhisharma404 reopened this Aug 7, 2018
@mariobehling mariobehling requested review from sansyrox, betterclever, cweitat and prateekiiest and removed request for sansyrox August 9, 2018 01:31
try:
identity_json = parsed_dict['session']['identity']
identity = Identity(identity_json)
session = Session(identity)
except KeyError:
session = None

return QueryResponse(answer,parsed_dict, session)
if parsed_dict['answers']:
Copy link
Member

Choose a reason for hiding this comment

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

Check if the len of parse_dict[answers] is not zero
and update accordingly

Copy link
Author

Choose a reason for hiding this comment

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

This returns false if it's empty, it works either way. Should I change it?

@prateekiiest
Copy link
Member

Please mention the issue you are working on by linking to the issue number

Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

@abhisharma404 , please create / refer the issue that you are solving prior to making a PR

@mariobehling
Copy link
Member

@abhisharma404 Which issue are you solving? Where is the bug reported?

@prateekiiest
Copy link
Member

The corresponding issue is this #66 @mariobehling

@abhisharma404
Copy link
Author

I'm sorry I was a bit busy with my exams. The issue that I had worked on is #66.
P.S. I'm new to git...so please bare my trivial questions! :)

Copy link
Member

@sansyrox sansyrox left a comment

Choose a reason for hiding this comment

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

@prateekiiest @abhisharma404 , I believe this change will cause errors in SUSI Linux . As we expect an empty response there.

@prateekiiest
Copy link
Member

I believe you are right @stealthanthrax

@mariobehling
Copy link
Member

Should this be closed then?

@prateekiiest
Copy link
Member

prateekiiest commented Sep 2, 2018

Due to the current breaking, I think we cant incorporate this.

@Orbiter
Copy link
Contributor

Orbiter commented Feb 27, 2019

rejected by developers

@Orbiter Orbiter closed this Feb 27, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants