-
Notifications
You must be signed in to change notification settings - Fork 580
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
KeywordBear.py: Output appropriate message #1271
Conversation
Thanks for your contribution! Reviewing pull requests take really a lot of time and we're all volunteers. Please make sure you go through the following check list and complete them all before pinging someone for a review.
As you learn things over your Pull Request please help others on the chat and on PRs to get their stuff right as well! |
for result in annotation_bear_results: | ||
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') |
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.
Line is longer than allowed. (86 > 79)
LineLengthBear, severity NORMAL, section linelength
.
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') | ||
|
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.
Line contains following spacing inconsistencies:
- Trailing whitespaces.
SpaceConsistencyBear, severity NORMAL, section python
.
The issue can be fixed by applying the following patch:
--- a/bears/general/KeywordBear.py
+++ b/bears/general/KeywordBear.py
@@ -23,7 +23,7 @@
yield from result.contents.get('comments', [])
except Exception:
raise TypeError('Given Language in the input is not supported by KeywordBear')
-
+
def generate_diff(comments, file, filename,
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') | ||
|
||
|
||
|
||
def generate_diff(comments, file, filename, |
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.
E303 too many blank lines (3)'
PycodestyleBear (E303), severity NORMAL, section autopep8
.
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') | ||
|
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.
W293 blank line contains whitespace'
PycodestyleBear (W293), severity NORMAL, section autopep8
.
for result in annotation_bear_results: | ||
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') |
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.
E501 line too long (86 > 79 characters)'
PycodestyleBear (E501), severity NORMAL, section autopep8
.
Comment on 5c94a92. Body of HEAD commit contains too long lines. Commit body lines should not exceed 72 characters. GitCommitBear, severity NORMAL, section |
for result in annotation_bear_results: | ||
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') |
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.
The code does not comply to PEP8.
PEP8Bear, severity NORMAL, section autopep8
.
The issue can be fixed by applying the following patch:
--- a/bears/general/KeywordBear.py
+++ b/bears/general/KeywordBear.py
@@ -22,8 +22,8 @@
for result in annotation_bear_results:
yield from result.contents.get('comments', [])
except Exception:
- raise TypeError('Given Language in the input is not supported by KeywordBear')
-
+ raise TypeError(
+ 'Given Language in the input is not supported by KeywordBear')
def generate_diff(comments, file, filename,
5c94a92
to
6c77db6
Compare
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') | ||
|
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.
Line contains following spacing inconsistencies:
- Trailing whitespaces.
SpaceConsistencyBear, severity NORMAL, section python
.
The issue can be fixed by applying the following patch:
--- a/bears/general/KeywordBear.py
+++ b/bears/general/KeywordBear.py
@@ -23,7 +23,7 @@
yield from result.contents.get('comments', [])
except Exception:
raise TypeError('Given Language in the input is not supported by KeywordBear')
-
+
def generate_diff(comments, file, filename,
line, line_number, pos):
for result in annotation_bear_results: | ||
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') |
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.
Line is longer than allowed. (86 > 79)
LineLengthBear, severity NORMAL, section linelength
.
Comment on 6c77db6. Body of HEAD commit contains too long lines. Commit body lines should not exceed 72 characters. GitCommitBear, severity NORMAL, section |
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') | ||
|
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.
W293 blank line contains whitespace'
PycodestyleBear (W293), severity NORMAL, section autopep8
.
for result in annotation_bear_results: | ||
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') |
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.
E501 line too long (86 > 79 characters)'
PycodestyleBear (E501), severity NORMAL, section autopep8
.
for result in annotation_bear_results: | ||
yield from result.contents.get('comments', []) | ||
except Exception: | ||
raise TypeError('Given Language in the input is not supported by KeywordBear') |
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.
The code does not comply to PEP8.
PEP8Bear, severity NORMAL, section autopep8
.
The issue can be fixed by applying the following patch:
--- a/bears/general/KeywordBear.py
+++ b/bears/general/KeywordBear.py
@@ -22,8 +22,9 @@
for result in annotation_bear_results:
yield from result.contents.get('comments', [])
except Exception:
- raise TypeError('Given Language in the input is not supported by KeywordBear')
-
+ raise TypeError(
+ 'Given Language in the input is not supported by KeywordBear')
+
def generate_diff(comments, file, filename,
line, line_number, pos):
6c77db6
to
9b94b80
Compare
Comment on 9b94b80. Body of HEAD commit contains too long lines. Commit body lines should not exceed 72 characters. GitCommitBear, severity NORMAL, section |
9b94b80
to
d135f43
Compare
KeywordBear.py is not fully covered by the tests. You'll have to add tests for 100% coverage |
d135f43
to
5b0ab35
Compare
Run |
try: | ||
for result in annotation_bear_results: | ||
yield from result.contents.get('comments', []) | ||
except AttributeError: |
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.
check January 7, 2017 7:17 PM
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.
@aptrishu I think it's a lot simpler to use github as the main review UI, just copying the text from gitter makes it way easier for other reviewers to see what's going on here
5b0ab35
to
44d0cf8
Compare
29611c8
to
e12b1a7
Compare
@Shade5 Need help in writing test case |
e12b1a7
to
1ecdf12
Compare
Does |
Here is it @Shade5 |
So you need to write two tests. One that satisties the |
Yes I am trying to cover the log statements using assertLogs but they are failing. Can you have a look on test case I have wrriten |
How are you calling |
I think |
Any errors with |
I've imported logging in both Keyword.py and KeywordBearTest.py and |
1ecdf12
to
ea92914
Compare
ea92914
to
cda2c86
Compare
cda2c86
to
2308997
Compare
'AnnotationBear': [ | ||
annotation_bear_result_type( | ||
'coalang specification for anything not found.' | ||
) |
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.
We end the brackets/parantheses in the same line as the last element. See line 207 of this file.
|
||
text = ['# todo 123'] | ||
|
||
with execute_bear(self.uut, 'F', text, |
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.
Please use named arguments - they're easier to read. Every other test uses them and consistent is better :)
See line 211 of this file.
dependency_results=dep_results) as result: | ||
self.assertEqual(result[0].diffs, {}) | ||
self.assertEqual(result[0].affected_code[0].start.line, 1) | ||
self.assertEqual(len(result), 1) |
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.
this should probably be the first assert :3
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.
And there aren't any checks to make sure the message is logged? I mean, that's what the whole PR is about.
2308997
to
f548edd
Compare
@adtac Done with the changes. Is it mergeable now? 😃 |
yield from result.contents.get('comments', []) | ||
if isinstance(result.contents, str): | ||
logging.error(result.contents) | ||
yield [] |
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.
why do you yield an empty list? I'd just remove it.
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.
what if I'm expecting a list? not yielding a list might trigger an exception
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.
As it was yeilding an empty list when comments
key was not found in request.contents I thought I had to do the same in case when this execption arises.
Fine I'll remove it 👍
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.
uhm yield is not a return @adtac
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.
ah didn't realize the original was a yield from
, not a yield
Output appropriate message if the language given in input is not valid/not supported for KeywordBear Fixes coala#1256
f548edd
to
75ae1c9
Compare
ack 75ae1c9 |
@rultor merge |
Output appropriate message if the language given in input is not valid/not supported for KeywordBear
Fixes #1256