Minor code cleaning, formatted error messages #201
Conversation
|
I wasn't sure on the best approach. For now just cleaning it up for easier future updates.
I'll hit it again tonight and see what I can do. Sorry bout that ^^,
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Swapnil Agarwal <[email protected]>
Sent: Thursday, June 15, 2017 8:46:00 AM
To: swapagarwal/JARVIS-on-Messenger
Cc: Zachary Williamson; Author
Subject: Re: [swapagarwal/JARVIS-on-Messenger] Minor code cleaning, formatted error messages (#201)
@swapagarwal commented on this pull request.
I was expecting a PR to modularize the error messages. 😅
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#201 (review)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AZTGZKF3jkf1bfxLGpxr7FQLS0sKNiakks5sETWYgaJpZM4N6mSw>.
|
| @@ -0,0 +1,55 @@ | |||
|
|
|||
There was a problem hiding this comment.
This isn't a module. It's more of a helper class. Can be moved to modules/examples.py
Keep the base error in the module itself for now, and extract just the list of examples out. This should be used both for the error messages in src files, as well as in tests.
There was a problem hiding this comment.
Unsure, so asking for clarification:
Split into two files, one for error messages (error_msg.py) and one for examples (examples.py)?
I understand storing them a directory above, to use for both tests and src files.
There was a problem hiding this comment.
Let's make only one file now: examples.py. (error_msg.py can be created later if required; let the error messages be in the corresponding module itself, like it is now. Will visit it later)
examples file would have a list of sample queries for each of the modules, which will be used in both src and tests.
| help += '\n\nI\'m always learning, so do come back and say hi from time to time!' | ||
| help += '\nHave a nice day. :)' | ||
| helper = helper.replace('there', entities['sender']['first_name']) | ||
| helper += """ |
There was a problem hiding this comment.
Reset this file and others which are related to triple-quoting the strings. They can be picked up in a separate PR.
There was a problem hiding this comment.
Covered in the initial pull-request; I will include it with the next one,
There was a problem hiding this comment.
Remove these changes for now. Only keep the examples refactoring in this PR.
|
Also, rebase your work on the current master to remove conflicts. |
| @@ -0,0 +1,49 @@ | |||
| EXAMPLE_ANIME = """ | |||
| - Death Note anime, | |||
| - HKD to USD | ||
| - USD to EUR rate | ||
| - how much is 100 USD to INR""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - define comfort | ||
| - cloud definition | ||
| - what does an accolade mean?""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - paradise lyrics | ||
| - lyrics of the song hall of fame | ||
| - What are the lyrics to see you again?""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - batman movie | ||
| - iron man 2 movie plot | ||
| - What is the rating of happiness movie?""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - time in new york | ||
| - india time | ||
| - time at paris""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - sia videos | ||
| - videos by eminem | ||
| - video coldplay""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - tell me the weather in London | ||
| - weather Delhi | ||
| - What's the weather in Texas?""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| EXAMPLE_WIKI = """ | ||
| - wikipedia barack | ||
| - html wiki | ||
| - who is sachin tendulkar""" |
| LOCATION_ERROR = "I couldn't get the {} at the location you specified." + BASE_ERROR | ||
|
|
||
| EXAMPLE_ANIME = """ | ||
| - Death Note anime, |
| - HKD to USD | ||
| - USD to EUR rate | ||
| - how much is 100 USD to INR""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - define comfort | ||
| - cloud definition | ||
| - what does an accolade mean?""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - paradise lyrics | ||
| - lyrics of the song hall of fame | ||
| - What are the lyrics to see you again?""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - batman movie | ||
| - iron man 2 movie plot | ||
| - What is the rating of happiness movie?""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - time in new york | ||
| - india time | ||
| - time at paris""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - sia videos | ||
| - videos by eminem | ||
| - video coldplay""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - tell me the weather in London | ||
| - weather Delhi | ||
| - What's the weather in Texas?""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| EXAMPLE_WIKI = """ | ||
| - wikipedia barack | ||
| - html wiki | ||
| - who is sachin tendulkar""" |
| - show a random xkcd comic | ||
| - latest news | ||
| - paradise lyrics | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| - hymn for the weekend song | ||
| - linkin park songs | ||
| - play hotel california""" | ||
|
|
There was a problem hiding this comment.
W293 blank line contains whitespace
| request += '\n - Report a bug (I couldn\'t handle the query and/or gave unexpected results), by including your search query and the expected result.' | ||
| request = """Kindly use the following buttons to: | ||
| - Request a new feature, by including some sample queries and their expected results. | ||
| - Report a bug (I couldn't handle the query and/or gave unexpected results), by including your search query and the expected result.""" |
There was a problem hiding this comment.
E501 line too long (139 > 120 characters)
| error_message += '\n - time in new york' | ||
| error_message += '\n - india time' | ||
| error_message += '\n - time at paris' | ||
| error_message = LOCATION_ERROR.format('time') + EXAMPLE_TIME |
There was a problem hiding this comment.
F821 undefined name 'LOCATION_ERROR'
F821 undefined name 'EXAMPLE_TIME'
| error_message += '\n - sia videos' | ||
| error_message += '\n - videos by eminem' | ||
| error_message += '\n - video coldplay' | ||
| error_message = QUERY_ERROR.format('videos') + EXAMPLE_VIDEOS |
There was a problem hiding this comment.
F405 'EXAMPLE_VIDEOS' may be undefined, or defined from star imports: templates.generic
|
@williamsonz Could we fix these, please. |
No description provided.