These are chat archives for coala/coala-bears

5th
Mar 2017
Yash Nisar
@yash-nisar
Mar 05 2017 07:33
Is using verify_local_bear for tests a bad practise ?
Meet Mangukiya
@meetmangukiya
Mar 05 2017 07:34
I consider it
Saurav Singh
@damngamerz
Mar 05 2017 07:34
check_results is more solid
Yash Nisar
@yash-nisar
Mar 05 2017 07:34
A majority of our bears use verify_local_bear :worried:
Very few have check_results
Saurav Singh
@damngamerz
Mar 05 2017 07:36
only 3 :stuck_out_tongue: have check_results
Yash Nisar
@yash-nisar
Mar 05 2017 07:36
4, if theLicenseCheckBear gets merged. :P
Do I have to rewrite the tests if I have used verify_local_bear for another bear ?
Saurav Singh
@damngamerz
Mar 05 2017 07:37
5 if my PR gets merged i.e PHPStanBear :stuck_out_tongue:
Yash Nisar
@yash-nisar
Mar 05 2017 07:37
haha
We're moving towards check_results :clap:
Saurav Singh
@damngamerz
Mar 05 2017 07:38
actually maintainers are trying to move away from verify_local_bear
so use check_results
Yash Nisar
@yash-nisar
Mar 05 2017 07:39
Okay, I will have to rewrite the tests then :worried:
Lets see if they approve it
Meet Mangukiya
@meetmangukiya
Mar 05 2017 07:39
It is okay if you have got everything covered
from next PR don't use it :)
Yash Nisar
@yash-nisar
Mar 05 2017 07:40
Alright, thanks @meetmangukiya
George Joseph
@Shade5
Mar 05 2017 08:10
Can someone assign me to this? coala/coala-bears#957
Sanket Dasgupta
@SanketDG
Mar 05 2017 14:26
@Makman2 you said you tried to run the current docstylebear on the codebase
can you help me with the glob?
can't get the glob right
Mischa Krüger
@Makman2
Mar 05 2017 14:35
Just executed it on coala itself @SanketDG
:+1:
Sanket Dasgupta
@SanketDG
Mar 05 2017 14:35
so all files in the coala codebase?
Mischa Krüger
@Makman2
Mar 05 2017 14:41
yep
just added a "Docs" section for DocumentationStyleBear relevant sections and executed it
Sanket Dasgupta
@SanketDG
Mar 05 2017 14:46
uh okay
Sanket Dasgupta
@SanketDG
Mar 05 2017 15:35
@Makman2 I ran the bear on some files, and it kind of works well, there are some gotcha cases, that we need to lay out, but other than that, I dont see any issues :smile:
Mischa Krüger
@Makman2
Mar 05 2017 15:35
:+1: :)
Sanket Dasgupta
@SanketDG
Mar 05 2017 15:36
any special cases you want me to know about? otherwise close to merge :P
Mischa Krüger
@Makman2
Mar 05 2017 15:41
actually there are problems, some patches are definitely wrong
and do cut some lines away or so
don't remember that well, but the parsing needs to be improved
@SanketDG
Sanket Dasgupta
@SanketDG
Mar 05 2017 15:45
uh okay, I'll run on all files then
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:15
one major issue are codeblocks
another would be multi-line strings that are not docstrings
and the fact that the wrapping length gets preserved, leading to unused space
Other than that I dont really see erroneous lintering :worried:
I will admit I didn't take a very detailed look, but from a first look it looks pretty okay
@Makman2 ^
Mischa Krüger
@Makman2
Mar 05 2017 16:27
@SanketDG
-    :raises ValueError:
+        :raises ValueError:
         Raised when illegal options are specified.
that's bad :)
this is bad too:
         - ``None``: Define your own format by overriding ``process_output``.
-          Overriding ``process_output`` is then mandatory, not specifying it
-          raises a ``ValueError``.
+        Overriding ``process_output`` is then mandatory, not specifying it
+        raises a ``ValueError``.
         - ``'regex'``: Parse output using a regex. See parameter
-          ``output_regex``.
+        ``output_regex``.
         - ``'corrected'``: The output is the corrected of the given file. Diffs
-          are then generated to supply patches for results.
+        are then generated to supply patches for results.
and sadly this one is a no-go:
@@ -32,9 +32,8 @@ class Emptiness:
         example = '(no text at all)'
         example_language = 'English'
         importance_reason = """
-        An empty commit message shows the lack of documentation for your
-        change.
-        """
+        importance_reason = age shows the lack of documentation for your
+        importance_reason = """
         fix_suggestions = 'Write a commit message.'
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:30
how do you want to handle the last case?
these are just multi-line strings
Mischa Krüger
@Makman2
Mar 05 2017 16:30
we need to exclude them^^
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:30
how would you differentiate?
one solution would be just to take in account context-specific docstrings
Mischa Krüger
@Makman2
Mar 05 2017 16:31
that's the actual problem
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:31
like at start of the class
Mischa Krüger
@Makman2
Mar 05 2017 16:31
yeah that's harder
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:31
after the method
Mischa Krüger
@Makman2
Mar 05 2017 16:31
I think we should just require docs to have nothing before them:
"""
docstring
"""

x = """
multiline string
"""
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:32
uh yes this would cover most cases :+1:
call.function("""hello""")
corner-case^^, but nobody writes code this way :P
Mischa Krüger
@Makman2
Mar 05 2017 16:33
would also work
worse is this one:
call.function(
    """
    hello
    """
)
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:33
:O
Mischa Krüger
@Makman2
Mar 05 2017 16:33
but right nobody writes code like this ;)
problem is that in python those """ are normal strings and valid expressions
which results in collisions sometimes
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:34
yup
Mischa Krüger
@Makman2
Mar 05 2017 16:35
though I think "nothing-before" is a good approximation we can use for now
ideally we would use coast for this to detect docstrings attached to stuff
though this is really damn hard then, as for other languages, comments are actually comments and not expressions
and then we start to mix things
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:35
yup, not generic
Mischa Krüger
@Makman2
Mar 05 2017 16:35
uncool, let's not do this for the time being :)
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:35
accounting for exceptions would be easy, the work is already done
Mischa Krüger
@Makman2
Mar 05 2017 16:36
oh right, in fact we could apply special parsing for python :3
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:36
yup
Mischa Krüger
@Makman2
Mar 05 2017 16:36
anyway, let's do it simpler for now :)
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:36
the second one is about ignoring rst
Makman2 @Makman2 is getting a shower :)
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:38
thanks, got enough for my next iteration :+1:
Mischa Krüger
@Makman2
Mar 05 2017 16:38
:D
Yash Nisar
@yash-nisar
Mar 05 2017 16:53
Can anyone help me with http://paste.ubuntu.com/24118802/ ?
Lists differ where the length of both the diffs is equal so I'm unable to figure out
Sanket Dasgupta
@SanketDG
Mar 05 2017 16:55
@yash-nisar replace your origin in Result.from_values() to the bear instance
most likely it will be self.uut
Yash Nisar
@yash-nisar
Mar 05 2017 16:56
Yup, it is self.uut
check_results is difficult to debug :worried:
Sanket Dasgupta
@SanketDG
Mar 05 2017 17:00
Can you open a PR though? That way we can pull it in for further testing
Yash Nisar
@yash-nisar
Mar 05 2017 17:02
or should I move to check_validity ?
Sanket Dasgupta
@SanketDG
Mar 05 2017 17:03
Let's keep it to check_results
Yash Nisar
@yash-nisar
Mar 05 2017 17:03
Okay :+1:
Thanks @SanketDG