Improving the dev workflow: Milestone 1.1.3 Custom pylint checks #4967

apb7 · 2018-05-22T18:14:06Z

Explanation

This PR adds a custom checker for docstrings. The Google Style Docstring checker provided by Pylint has been modified according to the docstring style followed at Oppia.

Checks enabled:

redundant-yields-doc
missing-return-doc
missing-return-type-doc
missing-yield-doc
missing-yield-type-doc
missing-param-doc
missing-type-doc
differing-param-doc
differing-type-doc

Checks disabled:

multiple-constructor-doc
missing-raises-doc
redundant-returns-doc

Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes.
The PR explanation includes the words "Fixes #bugnum: ...".
The linter/Karma presubmit checks have passed.
- These should run automatically, but if not, you can manually trigger them locally using python scripts/pre_commit_linter.py and bash scripts/run_frontend_tests.sh.
The PR is made from a branch that's not called "develop".
The PR follows the style guide.
The PR is assigned to an appropriate reviewer.
- If you're a new contributor, please ask on Gitter for someone to assign a reviewer.
- If you're not sure who the appropriate reviewer is, please assign to the issue's "owner" -- see the "talk-to" label on the issue.

Update Fork

Update fork

Update Fork

Update fork

codecov-io · 2018-05-22T19:36:13Z

Codecov Report

Merging #4967 into develop will increase coverage by 0.27%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #4967      +/-   ##
===========================================
+ Coverage    44.56%   44.84%   +0.27%     
===========================================
  Files          387      389       +2     
  Lines        23345    23510     +165     
  Branches      3800     3830      +30     
===========================================
+ Hits         10403    10542     +139     
- Misses       12942    12968      +26

Impacted Files	Coverage Δ
...ensions/objects/templates/NumberWithUnitsEditor.js	`4.54% <0%> (ø)`
...ead/domain/objects/NumberWithUnitsObjectFactory.js	`96.5% <0%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec10dc6...5769c5f. Read the comment docs.

seanlip

Hi @apb7, I answered your question, but also noticed a lot of issues. I haven't read the whole PR super carefully and haven't identified all instances of the issues, so please take a look at the rest of it as well and make changes as needed. Thanks.

/cc @kevinlee12

seanlip · 2018-05-24T07:05:33Z

utils.py

@@ -631,6 +631,9 @@ def _get_short_language_description(full_language_description):

    Args:
        full_language_description: str. Short description of the language.
+
+    Returns:
+        full_language_description: str. Short description of the language.


"Returns" shouldn't have a varname.

seanlip · 2018-05-24T07:06:08Z

core/controllers/base.py

@@ -211,6 +211,9 @@ def dispatch(self):
        Raises:
            Exception: The CSRF token is missing.
            UnauthorizedUserException: The CSRF token is invalid.
+
+        Returns:


Here and elsewhere: don't need Returns if there's nothing to return.

@seanlip: This checker mandates that if the function body has a return statement, it should be a part of the docstring even if it returns None. Therefore to enable the checker, I have added this. Also, I had discussed this with @kevinlee12 and have mentioned this in the PR explanation under the header Important Points. Thanks!

Ah, sorry, I didn't read the PR explanation since I started looking only at the specific comment you linked me to. Thanks for the pointer.

I think the standard in our codebase is to exclude the return statement if nothing is returned. Otherwise it's just cruft. Can we do that?

Also can we include the special cases *args and **kwargs in the regex?

For the former question, the checker works as follows:

It checks the function definition, that is, the parameters, raises, returns and yields parts.

If they ( parameters, raises, returns, and yields) are present, then it checks them in the corresponding function docstring.
Therefore if we use return in the function body, we need to have a docstring part corresponding to it.
For the latter part, we can try do so.

For the first point: can the checker be modified to detect a return statement that is "return {{something}}" instead?

Can we possibly follow this convention? What do you suggest?
Thanks!

See if you can do some investigation per my previous comment: can we detect "return {{something}}" and trigger on that, rather than "return"? If so, great; if not, why not?

Okay, let me look into this.

Actually, the problem is not with return. These statements do not require docstrings. This function actually has a return statement:

return self.handle_exception(e, self.app.debug)

The handle_exception function does not return anything other than None. ~~Therefore this statement can be safely removed.~~

seanlip · 2018-05-24T07:06:20Z

core/controllers/base.py

@@ -572,6 +575,9 @@ def is_csrf_token_valid(cls, user_id, token):
        Args:
            user_id: str. The user_id to validate the CSRF token against.
            token: str. The CSRF token to validate.
+
+        Returns:
+            bool.


Here and elsewhere, an explanation of what is being returned is needed

Done (here and elsewhere).

seanlip · 2018-05-24T07:06:30Z

core/controllers/base.py

@@ -440,7 +443,7 @@ def handle_exception(self, exception, unused_debug_mode):
        """Overwrites the default exception handler.

        Args:
-            exception: The exception that was thrown.
+            exception: exception. The exception that was thrown.


I think the class name (and therefore the type) is Exception

seanlip · 2018-05-24T07:07:11Z

core/domain/acl_decorators.py

@@ -45,6 +45,10 @@ def test_can_play(self, exploration_id, **kwargs):

        Args:
            exploration_id: str. The exploration id.
+            kwargs: keyword_arguments.


Here and elsewhere: Keyword arguments.

(It's not a python variable, so you don't need to refer to it in snake case.)

This will require a modification of the regex since it allows only non-space characters for the argument type.

Oh, you're using keyword_arguments as a type. It's not a type. Use * instead.

seanlip · 2018-05-24T07:09:59Z

core/domain/stats_jobs_continuous.py

@@ -103,6 +103,9 @@ def reduce(key, stringified_values):
                <exploration_id>:<exploration_version>:<state_name>
            stringified_values: list(str). A list of stringified_values of the
                submitted answers.
+
+        Yields:
+            Error.


This isn't useful at all. When is it yielded? Under what circumstances? You will need to read and understand the method to find out.

Docstrings should always be written to help the person reading the code. If we start just writing them just for the sake of it then it's a bit pointless.

Btw, for this case, where is the "Error" that's being yielded? As far as I can tell all the yields are just str.

So the docstring should just be:

Yields: str. One of the following strings: - ... - ...

Please check the others too. It's really not a great idea to introduce docstrings that have the wrong types since that's only going to confuse devs. If you are unsure about types, ask me.

seanlip · 2018-05-24T07:10:34Z

core/domain/user_jobs_continuous.py

-                as exp_models.ExplorationModel.
-            activity_ids_list: A list of activity IDs (such as exploration IDS)
-                for which the latest commits will be retrieved.
+            activity_model_cls: storage_layer_object. The storage layer


Nope, the type is wrong. We have no type called "storage_layer_object".

Should this be * since activity_model_cls can be one of exp_models.ExplorationModel of collection_models.CollectionModel?

It should be ExplorationModel|CollectionModel.

seanlip · 2018-05-24T07:10:39Z

core/domain/user_jobs_continuous.py

@@ -375,7 +376,7 @@ def _handle_incoming_event(cls, active_realtime_layer, event_type, *args):
                triggered and the total play count is incremented. If he/she
                rates an exploration, an event of type `rate` is triggered and
                average rating of the realtime model is refreshed.
-            *args:
+            args: arguments.


arguments is not a type

seanlip · 2018-05-24T07:10:59Z

core/domain/user_jobs_continuous.py

-            'average_ratings': float. Average of average ratings across all
-                explorations.
+        Returns:
+            dict. with the following keys:


dict. This has the following keys: ...

(Use complete sentences please.)

seanlip · 2018-05-24T07:13:26Z

scripts/_check_docs_utils.py

@@ -0,0 +1,718 @@
+# coding: utf-8


Not keen on mixing up third-party code in our regular codebase like this.

Can we put this in third_party instead and refer to it somehow, swapping out methods (just for the duration of the linter run) if needed?

We have third_party/ in .gitignore. Where should I put it then?

I have modified .gitignore to allow _check_docs_utils.py which has been placed in python-custom-checks. Thanks!

seanlip · 2018-05-24T08:51:44Z

core/domain/email_manager.py

@@ -224,6 +224,9 @@ def _send_email(
            the email.
        reply_to_id: str or None. The unique reply-to id used in reply-to email
            address sent to recipient.
+
+    Returns:
+        None.


FYI, I just noticed that this function doesn't actually need to return the result of run_in_transaction() on line 269. I think we can drop the return (and modify other stuff accordingly), if it helps.

This might be applicable for some other functions too.

So here can we remove all return statements from lines 244, 256 and 269?

That breaks the logic. So, no.

(Make sure you read the functions carefully and understand what they're doing. The fixes needed to bring the codebase into compliance with the linter are sweeping and should never be done cookie-cutter. There is a chance of breaking things here.)

Got it. Let me look into the "None" issue.

apb7

I have checked and removed all return statements which were actually equivalent to None.

apb7 · 2018-05-24T16:14:58Z

Hi @seanlip! The backend tests are failing. The traceback:

======================================================================
FAIL: test_non_logged_in_users_cannot_create_threads_and_messages (core.controllers.feedback_test.FeedbackThreadPermissionsTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/apurv/dev/oppia/core/controllers/feedback_test.py", line 105, in test_non_logged_in_users_cannot_create_threads_and_messages
    }, self.csrf_token, expect_errors=True, expected_status_int=401)
  File "/home/apurv/dev/oppia/core/tests/test_utils.py", line 421, in post_json
    upload_files)
  File "/home/apurv/dev/oppia/core/tests/test_utils.py", line 432, in _send_post_request
    self.assertEqual(json_response.status_int, expected_status_int)
AssertionError: 302 != 401

---------------------------------------------------------------------
FAILED    core.controllers.feedback_test: 0 errors, 1 failures

apb7 · 2018-05-24T16:36:57Z

core/controllers/base.py

@@ -244,7 +247,7 @@ def dispatch(self):
                    '%s: payload %s',
                    e, self.payload)

-                self.handle_exception(e, self.app.debug)
+                return self.handle_exception(e, self.app.debug)


Found out the problem. This return is necessary. We cannot remove it although handle_exception returns None.

Yes, because execution continues going onward. An alternative is to do

self.handle_exception(e, self.app.debug) return

seanlip

Hi @apb7, thanks! Made a couple comments; @kevinlee12 can you please take a closer look to verify correctness of docstrings and type information, and that no functional changes have been introduced in the core code?

Also, @apb7 -- is the only thing remaining to modify the checks so that they don't trigger on "return" without something actually being returned?

seanlip · 2018-05-24T18:49:53Z

.gitignore

+# The pre_commit_linter.py script uses _check_docs_utils.py placed
+# in python-custom-checks directory under third_party directory.
+# Therefore it is not ignored.
+!third_party/python-custom-checks


Sorry, no, this is not what I meant. I actually meant to locate the source file in third_party so that we do not have to ship it on github. Then modify the install scripts to download whatever is needed (pylint etc., or that specific file) and refer to it there.

That way, we don't ship/maintain code that is not ours (which is kind of the point of third-party in the first place).

Happy to chat about this if anything's not clear.

Done as discussed. Thanks!

seanlip · 2018-05-24T18:52:47Z

core/domain/stats_jobs_continuous.py

@@ -103,6 +103,9 @@ def reduce(key, stringified_values):
                <exploration_id>:<exploration_version>:<state_name>
            stringified_values: list(str). A list of stringified_values of the
                submitted answers.
+
+        Yields:
+            Error.


Btw, for this case, where is the "Error" that's being yielded? As far as I can tell all the yields are just str.

So the docstring should just be:

Yields: str. One of the following strings: - ... - ...

Please check the others too. It's really not a great idea to introduce docstrings that have the wrong types since that's only going to confuse devs. If you are unsure about types, ask me.

seanlip · 2018-05-24T18:55:24Z

core/controllers/base.py

@@ -244,7 +247,7 @@ def dispatch(self):
                    '%s: payload %s',
                    e, self.payload)

-                self.handle_exception(e, self.app.debug)
+                return self.handle_exception(e, self.app.debug)


Yes, because execution continues going onward. An alternative is to do

self.handle_exception(e, self.app.debug) return

apb7 · 2018-05-25T01:37:06Z

@seanlip: All review comments have been addressed including the no "return" problem. Also, the backend tests were failing. I have tried to fix them too. Thanks!

seanlip

Thanks @apb7, this looks like a much better approach IMO.

I've skimmed and made a few comments. I'm going to pass the rest of this review to @kevinlee12 (in terms of verifying varnames, types, descriptions, etc.).

seanlip · 2018-05-25T01:39:43Z

scripts/_check_docs_utils.py

@@ -0,0 +1,190 @@
+# coding: utf-8


To double-check: all code in this file was originally written by us?

Yes. The initial helper functions are similar but modified. I have changed the docstrings as well.

I'm confused what you mean by "similar but modified", can you elaborate?

The pylint functions check for all Docstrings style. This has been modified to check only for our modified Google Docstrings style.

seanlip · 2018-05-25T01:41:55Z

scripts/custom_lint_checks.py

-import os
-import sys
-
+import _check_docs_utils as utils


I don't think we need to follow their convention here. We can just call our file docstrings_checker.py or similar and do import docstrings_checker.

In general try to avoid aliasing (import X as Y) where possible.

seanlip · 2018-05-25T01:42:42Z

scripts/pre_commit_linter.py

        and filename.endswith('.py')]
+    # The file _check_docs_utils.py has to be excluded from this check


But now this file is under our control, right? So we can do it according to our code style.

On removing this, I still get an error:

/home/apurv/dev/oppia/./scripts/docstrings_checker.py --> Line 171: Multiline docstring should end with a new line. /home/apurv/dev/oppia/./scripts/docstrings_checker.py --> Line 183: Multiline docstring should end with a new line.

I looked into the working of _check_docstrings function and found out that it checks for """ at the start and end to identify multiline docstrings. Since the regex here also spans multiple lines, it produces the above error. Therefore we still require to disable this check for the file. Thanks!

No, we should not make a habit of disabling checks like this (in general). There are ways around this, e.g. you could write the multiline string a different way:

[ '...', '...' ].join('\n')

But, at worst, you would disable just the multiline docstring check for those two lines. It does not warrant disabling lint checks for the entire file.

Fixed, used ''' instead if """. Done!

seanlip · 2018-05-25T01:43:18Z

scripts/pre_commit_linter.py

@@ -821,8 +821,14 @@ def _check_docstrings(all_files):
    summary_messages = []
    files_to_check = [
        filename for filename in all_files if not
-        any(fnmatch.fnmatch(filename, pattern) for pattern in EXCLUDED_PATHS)
+        (any(


I don't think you need the outer parens. Didn't this and the next line all fit in one line before?

seanlip · 2018-05-25T01:43:57Z

scripts/install_third_party.py

@@ -201,8 +201,8 @@ def test_manifest_syntax(dependency_type, dependency_dict):

    Display warning message when there is an error and terminate the program.
    Args:
-      dependency_type: dependency download format.
-      dependency_dict: manifest.json dependency dict.
+      dependency_type: dependency_download_format.


dependency_download_format is not a type. Could you please go through all your changes here and make sure that the types and descriptions are correct?

Should the type here be "*"?

I don't know offhand. You'll need to read the code and investigate what the possible values for this argument are.

I think it should be str since test_manifest_syntax is called in validate_manifest. validate_manifest passes it download_format which is separated from dependencies. dependencies are separated from manifest_data, a JSON object.
@kevinlee12: Can you please see this once? Thanks!

kevinlee12

Hi @apb7, I left some more comments, ptal!

kevinlee12 · 2018-05-25T02:53:28Z

core/domain/email_manager.py

@@ -263,7 +263,7 @@ def _send_email_in_transaction():
            recipient_id, recipient_email, sender_id, sender_name_email, intent,
            email_subject, cleaned_html_body, datetime.datetime.utcnow())

-    return transaction_services.run_in_transaction(_send_email_in_transaction)
+    transaction_services.run_in_transaction(_send_email_in_transaction)


Why was the return removed?

@kevinlee12: Please refer to this discussion.

Okay, thanks!

kevinlee12 · 2018-05-25T02:53:39Z

core/domain/email_manager.py

@@ -312,7 +312,7 @@ def _send_bulk_mail_in_transaction(instance_id=None):
            instance_id, recipient_ids, sender_id, sender_name_email, intent,
            email_subject, cleaned_html_body, datetime.datetime.utcnow())

-    return transaction_services.run_in_transaction(
+    transaction_services.run_in_transaction(


kevinlee12 · 2018-05-25T02:53:49Z

core/domain/email_manager.py

@@ -931,6 +931,6 @@ def send_test_email_for_bulk_emails(tester_id, email_subject, email_body):
    """
    tester_name = user_services.get_username(tester_id)
    tester_email = user_services.get_email_from_user_id(tester_id)
-    return _send_email(
+    _send_email(


kevinlee12 · 2018-05-25T03:05:41Z

core/domain/stats_jobs_continuous.py

+                    Occurs when there is not exactly one interaction ID
+                    for each exploration and version.
+                - Expected at least one item ID for exploration:
+                    Occurs when there is not atleast one Item ID for


at least (add space between the words)

kevinlee12 · 2018-05-25T03:25:53Z

core/domain/user_services.py

@@ -110,6 +110,9 @@ def __init__(
                user last edited an exploration.
            profile_picture_data_url: str or None. User uploaded profile
                picture as a dataURI string.
+            default_dashboard: str|None. The dashboard the user wants.


Rephrase to: The default dashboard of the user.

kevinlee12 · 2018-05-25T03:26:24Z

core/domain/user_services.py

@@ -110,6 +110,9 @@ def __init__(
                user last edited an exploration.
            profile_picture_data_url: str or None. User uploaded profile
                picture as a dataURI string.
+            default_dashboard: str|None. The dashboard the user wants.
+            creator_dashboard_display_pref: str. The creator dashboard


The creator dashboard of the user.

kevinlee12 · 2018-05-25T03:27:21Z

core/platform/email/gae_email_services.py

@@ -92,7 +92,8 @@ def send_bulk_mail(
    Args:
        sender_email: str. The email address of the sender. This should be in
            the form 'SENDER_NAME <SENDER_EMAIL_ADDRESS>'.
-        recipient_email: str. The email address of the recipient.
+        recipient_emails: list(str). The list of email addresses


List of recipients' email addresses.

kevinlee12 · 2018-05-25T03:28:49Z

core/storage/classifier/gae_models.py

@@ -290,6 +290,9 @@ def create_multi(cls, job_exploration_mappings):
        Args:
            job_exploration_mappings: list(TrainingJobExplorationMapping). The
                list of TrainingJobExplorationMapping Domain objects.
+


(Since Github won't let me comment there)

Please remove the space between new and TrainingJobExplorationMappingModel.

kevinlee12 · 2018-05-25T03:32:13Z

scripts/_check_docs_utils.py

@@ -0,0 +1,190 @@
+# coding: utf-8


I'm confused what you mean by "similar but modified", can you elaborate?

apb7 · 2018-05-25T04:44:03Z

@kevinlee12: I have addressed all the review comments. Can you please take another look?
Thanks!

kevinlee12

Hi @apb7, some more nits, ptal!

kevinlee12 · 2018-05-25T04:59:31Z

scripts/custom_lint_checks.py

@@ -143,6 +149,405 @@ def process_module(self, node):
                        'no-break-after-hanging-indent', line=line_num + 1)


+# This class has been derived from


Modify to The following class was derived from

kevinlee12 · 2018-05-25T05:01:13Z

scripts/custom_lint_checks.py

@@ -143,6 +149,405 @@ def process_module(self, node):
                        'no-break-after-hanging-indent', line=line_num + 1)


+# This class has been derived from
+# https://github.com/PyCQA/pylint/blob/master/pylint/extensions/docparams.py#L26.


For the link, change it to the following one:
https://github.com/PyCQA/pylint/blob/377cc42f9e3116ff97cddd4567d53e9a3e24ebf9/pylint/extensions/docparams.py#L26

This is so that we chain this to a particular version that we are using.

apb7 · 2018-05-25T05:33:53Z

@kevinlee12: ptal! Thanks.

kevinlee12

lgtm, thanks @apb7!

kevinlee12 · 2018-05-25T16:32:57Z

core/domain/exp_services.py

@@ -1583,6 +1587,8 @@ def get_next_page_of_all_non_private_commits(
        urlsafe_start_cursor: str. If this is not None, then the returned
            commits start from cursor location. Otherwise they start from the
            beginning of the list of commits.
+        max_age: datetime.timedelta. The maximum age to which all non private
+            commits are fetch from the ExplorationCommitLogEntry.


...are fetched...

Dismissing since I'm taking over final review.

apb7 added 23 commits February 19, 2018 02:48

Merge pull request #13 from oppia/develop

28f20ed

Update Fork

Merge pull request #14 from oppia/develop

cdc2856

Update Fork

Merge pull request #15 from oppia/develop

af636ec

Update Fork

Merge pull request #16 from oppia/develop

860d44c

Update Fork

Merge pull request #19 from oppia/develop

0cdc197

Update Fork

Merge pull request #20 from oppia/develop

bf888ba

Update Fork

Merge pull request #21 from oppia/develop

5e546fd

Update fork

Merge pull request #22 from oppia/develop

9859cd2

Update fork

Merge pull request #23 from oppia/develop

e90fcde

Update fork

Merge pull request #24 from oppia/develop

b7b69fb

Update fork

Merge pull request #25 from oppia/develop

108bd8d

Update fork

Merge pull request #26 from oppia/develop

f165670

Update Fork

Merge pull request #27 from oppia/develop

759e4da

Update fork

Merge pull request #28 from oppia/develop

420df20

Update fork

Merge pull request #29 from oppia/develop

0fb552c

Update fork

Merge pull request #30 from oppia/develop

91dfed9

Update fork

Merge pull request #31 from oppia/develop

c1f8fea

Update fork

Add local check

8cf6257

Basic setup

dab5560

Fix basic lint errors

1a09342

lint fix

858c9fc

more lint fix

a604851

more lint fix

d4f2f8c

apb7 self-assigned this May 22, 2018

lint fix core/controllers

493ddab

apb7 added 4 commits May 23, 2018 02:03

partial lint fix core/domain

0a9ef41

partial lint fix core/domain()

a68d0d7

partial lint fix core/domain(2)

190d70d

lint fix core/domain

e5f6404

seanlip previously requested changes May 24, 2018

View reviewed changes

apb7 assigned apb7 and unassigned kevinlee12 May 24, 2018

seanlip reviewed May 24, 2018

View reviewed changes

apb7 added 3 commits May 24, 2018 17:58

Address all review comments except returns

9075e29

lint fix

9e66a21

fix yields

c616ecd

apb7 commented May 24, 2018

View reviewed changes

remove unnecessary returns

aa63883

Fix backend tests

8b354ca

apb7 commented May 24, 2018

View reviewed changes

seanlip reviewed May 24, 2018

View reviewed changes

apb7 added 4 commits May 25, 2018 06:36

Address all review comments

058b7ee

Add doc utils

3496860

Add yields line

dae48e7

lint fix

3cc5d33

seanlip reviewed May 25, 2018

View reviewed changes

kevinlee12 reviewed May 25, 2018

View reviewed changes

Review changes

f398be4

kevinlee12 suggested changes May 25, 2018

View reviewed changes

Review changes

5769c5f

apb7 assigned kevinlee12 and unassigned apb7 May 25, 2018

kevinlee12 approved these changes May 25, 2018

View reviewed changes

kevinlee12 merged commit b00ad88 into oppia:develop May 25, 2018

		and filename.endswith('.py')]
		# The file _check_docs_utils.py has to be excluded from this check

		@@ -143,6 +149,405 @@ def process_module(self, node):
		'no-break-after-hanging-indent', line=line_num + 1)


		# This class has been derived from

Improving the dev workflow: Milestone 1.1.3 Custom pylint checks #4967

Improving the dev workflow: Milestone 1.1.3 Custom pylint checks #4967

Conversation

apb7 commented May 22, 2018 • edited Loading

Explanation

Checks enabled:

Checks disabled:

Checklist

codecov-io commented May 22, 2018 • edited Loading

Codecov Report

seanlip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanlip May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apb7 May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apb7 left a comment

Choose a reason for hiding this comment

apb7 commented May 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanlip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

apb7 commented May 25, 2018

seanlip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanlip May 25, 2018 • edited Loading

apb7 commented May 22, 2018 •

edited

Loading

codecov-io commented May 22, 2018 •

edited

Loading

seanlip May 24, 2018 •

edited

Loading

apb7 May 24, 2018 •

edited

Loading

seanlip May 25, 2018 •

edited

Loading