Issue #3573: Resolve design problem of FileContentsHolder (step 1) #4468

timurt · 2017-06-19T13:39:30Z

MEZk · 2017-06-19T16:35:44Z

@timurt
CIs must be green.

Coverage threshold is not satisfied. Please, run mvn clean cobertura:check and mvn clean cobertura:cobertura.
IDEA's inspections violations should be resolved:

src/main/java/com/puppycrawl/tools/checkstyle/api
 FileProcessingResult.java (2)
43: FileProcessingResult() Assignment to Collection field this.text from parameter 'text'
44: FileProcessingResult() Assignment to Collection field this.messages from parameter 'messages'

Please, read http://checkstyle.sourceforge.net/idea.html#Inspections and run the inspections locally.

Had problem with Checker class
[checkstyle] [ERROR] /home/travis/build/checkstyle/checkstyle/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java:67:1: Class Fan-Out Complexity is 26 (max allowed is 25). [ClassFanOutComplexity]

@romani @rnveach
I think that this is a task for separate issue. Despite the fact that we increased class complexity by one, Checker has already been over-complicated.

MEZk · 2017-06-19T16:24:53Z

src/main/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResult.java

+import java.util.SortedSet;
+
+/**
+ * Represents the file results.


file processing results

MEZk · 2017-06-19T16:25:54Z

src/main/java/com/puppycrawl/tools/checkstyle/api/MessageDispatcher.java

@@ -41,7 +39,7 @@
    /**
     * Notify all listeners about the errors in a file.
     * @param fileName the audited file
-     * @param errors the audit errors from the file
+     * @param fileProcessingResult the audit errors from the file


Please, change javadoc

MEZk · 2017-06-19T16:37:37Z

src/main/java/com/puppycrawl/tools/checkstyle/Checker.java

@@ -308,12 +309,14 @@ private void processFiles(List<File> files) throws CheckstyleException {
     * @throws CheckstyleException if error condition within Checkstyle occurs.
     * @noinspection ProhibitedExceptionThrown
     */
-    private SortedSet<LocalizedMessage> processFile(File file) throws CheckstyleException {
+    private FileProcessingResult processFile(File file) throws CheckstyleException {


method javadoc should be corrected

MEZk · 2017-06-19T16:38:35Z

src/main/java/com/puppycrawl/tools/checkstyle/Checker.java

@@ -374,13 +377,13 @@ public void fireFileStarted(String fileName) {
     * Notify all listeners about the errors in a file.
     *
     * @param fileName the audited file
-     * @param errors the audit errors from the file
+     * @param fileProcessingResult the audit errors from the file


javadoc is not correct. Should be fixed.

MEZk · 2017-06-19T16:40:43Z

src/main/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResult.java

+     * Returns the contents of the file.
+     * @return an object containing the full text of the file
+     */
+    public FileText getText() {


The name of the method is cryptic. What does 'text' mean?

renamed to fileText

codecov-io · 2017-06-20T05:39:40Z

Codecov Report

Merging #4468 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff           @@
##           master   #4468   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         287     288    +1     
  Lines       15395   15410   +15     
  Branches     3502    3503    +1     
======================================
+ Hits        15395   15410   +15

Impacted Files	Coverage Δ
...awl/tools/checkstyle/api/FileProcessingResult.java	`100% <100%> (ø)`
.../java/com/puppycrawl/tools/checkstyle/Checker.java	`100% <100%> (ø)`	⬆️
...awl/tools/checkstyle/api/AbstractFileSetCheck.java	`100% <100%> (ø)`	⬆️
...rawl/tools/checkstyle/checks/TranslationCheck.java	`100% <100%> (ø)`	⬆️

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 3694f89...26d984a. Read the comment docs.

timurt · 2017-06-20T06:31:04Z

@MEZk
I am getting TravisCI errors

Connecting to www.oracle.com (www.oracle.com)|23.49.182.138|:80... connected.
HTTP request sent, awaiting response... 503 Service Unavailable
2017-06-20 05:40:47 ERROR 503: Service Unavailable.

MEZk · 2017-06-20T14:10:18Z

config/checkstyle_checks.xml

@@ -350,7 +350,7 @@
            DetailsAST, CheckstyleException, UnsupportedEncodingException, BuildException, ConversionException, FileNotFoundException, TestException"/>
    </module>
    <module name="ClassFanOutComplexity">
-        <property name="max" value="25"/>
+        <property name="max" value="26"/>


No, it is not allowed to change the config. There should be good reasons to change the config for all project. Please, revert.

but otherwise my PR won't pass CI

done, got error

@timurt
Please, create an issue. Explain, what your problem is there. Make a suppression, put a link to the issue into suppression file.

MEZk · 2017-06-20T14:20:34Z

src/test/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResultTest.java

+    @Test
+    public void testNonNullFileText() throws IOException {
+        final String charsetName = "ISO-8859-1";
+        final FileText fileText = new FileText(new File("src/test/resources/com/puppycrawl/tools/"


The test should have its own input file.

MEZk · 2017-06-20T14:28:37Z

src/main/java/com/puppycrawl/tools/checkstyle/Checker.java

        final String stripped = CommonUtils.relativizeAndNormalizePath(basedir, fileName);
        boolean hasNonFilteredViolations = false;
-        for (final LocalizedMessage element : errors) {
+        for (final LocalizedMessage element : fileProcessingResult.getMessages()) {


Please, extract fileProcessingResult.getMessages() to the local variable

rnveach · 2017-06-20T21:04:45Z

No, it is not allowed to change the config.
but otherwise my PR won't pass CI
done, got error

@timurt CI should be green for us to merge. You must suppress this error for the class.
See https://github.com/rnveach/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/Checker.java#L290 on how to suppress errors.

timurt · 2017-06-21T06:42:41Z

@rnveach
Done

MEZk

Please, fix new points.
Rename commit message to Issue #3573: resolve design problem of FileContentsHolder (step 1) to clearly indicate that this commit and PR are only the first step.

MEZk · 2017-06-21T07:41:39Z

src/main/java/com/puppycrawl/tools/checkstyle/Checker.java

        final String stripped = CommonUtils.relativizeAndNormalizePath(basedir, fileName);
        boolean hasNonFilteredViolations = false;
-        for (final LocalizedMessage element : errors) {
+        final SortedSet<LocalizedMessage> messages = fileProcessingResult.getMessages();
+        for (final LocalizedMessage element : messages) {


remove final modifier

rename element to something more appropriate

'auditErrors' name is ok?

remove final modifier

is it about final SortedSet<LocalizedMessage> messages = fileProcessingResult.getMessages(); ?
when I remove it, verification fails with Variable 'messages' should be declared final

@timurt
This line: for (final LocalizedMessage element : messages) {

MEZk · 2017-06-21T07:48:54Z

src/test/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResultTest.java

+    public void testNonNullFileText() throws IOException {
+        final String charsetName = "ISO-8859-1";
+        final FileText fileText = new FileText(new File("src/test/resources/com/puppycrawl/tools/"
+                + "checkstyle/api/InputFileProcessingResult.java"), charsetName);


You do not assert the content of the file directly in the test. Do we really need a separate file? Can we use temp folder and create the empty file there it it satisfied coverage threshold?

MEZk · 2017-06-21T07:52:25Z

src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractFileSetCheck.java

@@ -174,6 +174,6 @@ protected final void fireErrors(String fileName) {
        final SortedSet<LocalizedMessage> errors = messageCollector
                .getMessages();
        messageCollector.reset();
-        getMessageDispatcher().fireErrors(fileName, errors);
+        getMessageDispatcher().fireErrors(fileName, new FileProcessingResult(null, errors));


as we need to pass null as the constructor parameter it means that this is a problem in the design. Please, create one more constructor which receives one parameter and sets fileText to null. The constructor can be reused in the FileProcessingResult itself.

MEZk · 2017-06-21T09:34:39Z

@rnveach

CI should be green for us to merge.

Localized UTs are failing. Other jobs successfully finished.

@timurt
Ignore Travis CI failures for now.
See #4316 for details.

MEZk · 2017-06-22T06:33:31Z

@timurt
Ping

MEZk · 2017-06-22T07:14:35Z

src/main/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResult.java

+     *
+     * @param messages the sorted set of messages
+     */
+    public FileProcessingResult(SortedSet<LocalizedMessage> messages) {


Reuse this constructor in the previous one

Not previous constructor in this one?

Like this or not?

public FileProcessingResult(SortedSet<LocalizedMessage> messages) { this(null, messages); }

MEZk · 2017-06-22T07:16:49Z

src/main/java/com/puppycrawl/tools/checkstyle/Checker.java

        final String stripped = CommonUtils.relativizeAndNormalizePath(basedir, fileName);
        boolean hasNonFilteredViolations = false;
-        for (final LocalizedMessage element : errors) {
+        final SortedSet<LocalizedMessage> auditErrors = fileProcessingResult.getMessages();
+        for (LocalizedMessage element : auditErrors) {


Rename element to errorMessage, errors to errorMessages. We loop over messages, not audit events

MEZk · 2017-06-22T07:17:39Z

src/test/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResultTest.java

+
+public class FileProcessingResultTest {
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder();


Why is it public?

http://junit.org/junit4/javadoc/4.12/org/junit/Rule.html

Annotates fields that reference rules or methods that return a rule. A field must be public, not static, and a subtype of TestRule (preferred) or MethodRule. A method must be public, not static, and must return a subtype of TestRule (preferred) or MethodRule.

The filed should have been final. Now, I'm OK with the change.

MEZk · 2017-06-22T07:18:27Z

src/test/resources/com/puppycrawl/tools/checkstyle/api/InputFileProcessingResult.java

@@ -0,0 +1,5 @@
+package com.puppycrawl.tools.checkstyle.api;


We do not need this file anymore, please remove

MEZk · 2017-06-22T07:20:31Z

@timurt
Please fix TeamCity violations

timurt · 2017-06-22T11:39:47Z

@MEZk done

MEZk · 2017-06-22T15:08:51Z

@rnveach
I'm OK with the changes. Please, do final review.
Localized tests are still failing.

rnveach

@MEZk Please see my comments directed to you.
Only 1 real item for @timurt .

rnveach · 2017-06-22T15:32:34Z

src/main/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResult.java

+    public FileText getFileText() {
+        FileText result = null;
+        if (fileText != null) {
+            result = new FileText(fileText);


Why do we need a new instance? FileText is almost immutable except for lineBreaks, which is just populated on demand.
Can't we return the field as is?

@rnveach
Cobertura fails when I just return value, as the example I took FileContents#getText method

@timurt Please explain how cobertura is failing with just return fileText;.
There are no branches or such, and we know the method is already being executed.
Is coverage failing in another class?

@timurt
This point is not addressed.

rnveach · 2017-06-22T15:43:25Z

src/main/java/com/puppycrawl/tools/checkstyle/Checker.java

-            final AuditEvent event = new AuditEvent(this, stripped, element);
+        final SortedSet<LocalizedMessage> errorMessages = fileProcessingResult.getMessages();
+        for (LocalizedMessage errorMessage : errorMessages) {
+            final AuditEvent event = new AuditEvent(this, stripped, errorMessage);


Step 2
6. Add FileContents to audit event to make it available for filters. We will add AST root to FileContents later

@MEZk Shouldn't the event eventually accept FileProcessingResult?

@rnveach
Maybe, but this should not be done in this PR. I'll think about it.

@rnveach
You are right. AuditEvent should receive FileProcessingResult. Changed the plan.
#3573 (comment)

rnveach · 2017-06-22T15:45:51Z

src/main/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResult.java

+     *
+     * @param messages the sorted set of messages
+     */
+    public FileProcessingResult(SortedSet<LocalizedMessage> messages) {


@MEZk I think we should eventually remove this constructor as it doesn't contain the file contents. We are always working with a file, so I can't see why it shouldn't be populated (aside from any exception on file).

@rnveach
Yes, you are right. We always can instantiate FileText based on the file name and pass error messages and FileText to the constructor with two parameters. I did not see this fact. Hovewer, it will require to catch IOException from FileText constructor.

@timurt
Please, remove the constructor with one parameter. You always have the file path (file name) to instantiate FileText object and pass it to the constructor. So, both FileText object and the sorted set of localized messages can be instantiated.

it will require to catch IOException from FileText constructor.

@MEZk I'm not sure where you think IOException is coming from. Both constructors don't have an IOException being thrown right now and I am not talking about creating FileText in the constructor.
FileText is already created in TreeWalker https://github.com/checkstyle/checkstyle/pull/4468/files#diff-503cc093d3169abd5697cc8189f27f32R319 and passed around. We should be using that FileText everywhere somehow.

Both constructors don't have an IOException being thrown right now

FileText constructor throws IOException.

We will need to create FileText here. The constructor of FileText which receives the File object throws IOException.

@MEZk We pass FileText to all FileSets at https://github.com/checkstyle/checkstyle/pull/4468/files/8da878eaf14710f1d578ecadfa1caafe94512dc3#diff-503cc093d3169abd5697cc8189f27f32R321 . So TreeWalker should be able to hand it over to AbstractCheck without having to create a new one.
Since the class is still List<String> outside Checker, it depends on #3034 being done first.

@rnveach
DO we need 'file' as the first parameter of the process method? We already have the reference to the file in FileText.

@MEZk No we don't need it, but to accomplish this we would have to send the direct class FileText to all parties as they only receive List<String> as described in #3034.
It also seems slightly weird we would go to FileText for the file's name. we will soon also have FileProcessingResult with a lot of file related stuff.

Should we create a single class like CheckstyleFile. It will contain the file name, FileProcessingResult, FileText, and everything else related to the input and output of the process method? We could move the unnecessary fields from FileText into CheckstyleFile.
If so, let's create another issue on this.

The new issue was created #4527.

related to the input and output of the process method

Why do we need to include the output of the process method into CheckstyleFile? Can you explain in the issue ?
FileProcessingResult represents results (localized messages and information about the processed file). It can include CheckstyleFile which can be an argument of the process method.

rnveach · 2017-06-22T15:48:54Z

src/main/java/com/puppycrawl/tools/checkstyle/Checker.java

     */
    @Override
-    public void fireErrors(String fileName, SortedSet<LocalizedMessage> errors) {
+    public void fireErrors(String fileName, FileProcessingResult fileProcessingResult) {


@MEZk Should fileName be in FileProcessingResult?
It seems like they are fully conntected. FileText can also retrieve the File object, which can get it's name too.

@rnveach
Yes, we can get the name of the file from FileText which will become part of FileProcessingResult. Thus, we do not need the first parameter of fireErrors at all.

@timurt
Please, change the signature of fireErrors to

void fireErrors(FileProcessingResult fileProcessingResult)

@MEZk @rnveach
I'm stuck with one thing. Inside Checker#processFile, there is try-catch for IOException in a case specified file does not exist, it means message will be logged, but fileText will be null. If FileText object is null then we will not be able to extract fileName, absolutePath and etc. from it, for example inside Checker#fireErrors it will throw NPE.
Problem is fileName is used in various places even if the file does not exist
I can suggest to add additional filePath field to FileProcessingResult or left PRed implementation?

If FileText object is null then we will not be able to extract fileName, absolutePath and etc.

fileName will be a separate field inside FileProcessingResult, imo. Then we won't have to worry when FileText is null.

MEZk · 2017-06-24T19:15:12Z

@timurt
Timur, please start working on #3034 as it blocks this PR.

timurt · 2017-07-09T10:06:06Z

@MEZk
Rebased, fixed conflicts and points

timurt · 2017-07-09T10:25:48Z

@MEZk
TeamCity violation
80: CheckerTest Class 'CheckerTest' has too many dependencies (34 > 33)
How can I fix it?

Vladlis · 2017-07-09T16:23:14Z

@timurt
you should suppress this inspection, here are examples of other suppressions: https://github.com/checkstyle/checkstyle/search?utf8=%E2%9C%93&q=noinspection&type=

MEZk

@timurt
Please, suppress the TC violation. CheckerTest is a test class, it should be allowed to have many dependencies in the class.

MEZk · 2017-07-09T17:47:16Z

src/test/java/com/puppycrawl/tools/checkstyle/api/FileProcessingResultTest.java

+                new Object[] {"arg"}, null, getClass(), null));
+        final FileProcessingResult processingResult = new FileProcessingResult(
+                file.getAbsolutePath(), fileText, messages);
+        assertEquals(charsetName, processingResult.getFileText().getCharset().name());


Can we use just assertNotNull(processingResult.getFileText()) ?

I inserted this assert before assertEquals(charsetName, processingResult.getFileText().getCharset().name());
I see no reason to delete previous assert

Why do we need

assertEquals(charsetName, processingResult.getFileText().getCharset().name());

no actual reason, just to assert charset didn't change
I can remove it

@timurt
Please, remove. It is unnecessary. Judging by the name of the test the main idea is to check that FileText is not null when it is passed to FileProcessingResult.

timurt · 2017-07-09T18:34:49Z

@MEZk
Did I inspection suppression in wrong way?

/**
 * @noinspection ClassWithTooManyDependencies
 */
public class CheckerTest extends BaseCheckTestSupport {

romani · 2017-07-11T03:03:30Z

Did I inspection suppression in wrong way?

please reference name of inspection at https://github.com/checkstyle/checkstyle/blob/master/config/intellij-idea-inspections.xml#L1996

MEZk · 2017-07-11T16:38:02Z

config/intellij-idea-inspections.xml

@@ -1997,6 +1997,29 @@
    <inspection_tool class="SuperClassHasFrequentlyUsedInheritors" enabled="true" level="ERROR" enabled_by_default="true" />
    <inspection_tool class="SuperTearDownInFinally" enabled="false" level="ERROR" enabled_by_default="false" />
    <inspection_tool class="SuppressionAnnotation" enabled="true" level="ERROR" enabled_by_default="true">
+        <scope name="Tests" level="ERROR" enabled="true">


Why was the scope changed?

@MEZk
my previous tries were not successful so I tried to create another scope
I am just out of ideas

this is my previous configuration

<inspection_tool class="SuppressionAnnotation" enabled="true" level="ERROR" enabled_by_default="true"> <option name="myAllowedSuppressions"> <list> <option value="deprecation" /> <option value="unchecked" /> <option value="rawtypes" />  <option value="ProhibitedExceptionThrown" /> <option value="MismatchedQueryAndUpdateOfCollection" />  <option value="ProhibitedExceptionCaught" />  <option value="IfStatementWithTooManyBranches" />  <option value="ProhibitedExceptionThrown" />  <option value="NullArgumentToVariableArgMethod" />  <option value="ClassWithTooManyDependencies" /> </list> </option> </inspection_tool>

You can use scope for the certain option to make less changes to the config.
See https://github.com/timurt/checkstyle/blob/d8b5b759e859f66c8c5e932bce7eb5906e626912/config/intellij-idea-inspections.xml#L967

@MEZk
Is it correct?

<inspection_tool class="ClassWithTooManyDependencies" enabled="true" level="ERROR" enabled_by_default="true"> <scope name="Tests" level="ERROR" enabled="true">  <option name="limit" value="40" /> </scope> <option name="limit" value="33" /> </inspection_tool>

@SuppressWarnings("ClassWithTooManyDependencies") public class CheckerTest extends BaseCheckTestSupport {

scope Production means src/main?
scope Tests means src/test ?

…step 1)

romani

Sorry, that I did not find time to review your intentions far before.

Sorry, I am against this update:

romani · 2017-07-12T12:56:22Z

config/intellij-idea-inspections.xml

+        <scope name="Tests" level="ERROR" enabled="true">
+            <!-- we have to use it because number of dependencies of CheckerTest exceeds 33. -->
+            <option name="limit" value="40" />
+        </scope>


if there is problem in one file for whole rule, we do not change a rule, we do suppression in certain class.
please use javadoc tag suppression for CheckerTest.

romani · 2017-07-12T13:05:11Z

src/main/java/com/puppycrawl/tools/checkstyle/Checker.java

     */
    @Override
-    public void fireErrors(String fileName, SortedSet<LocalizedMessage> errors) {
+    public void fireErrors(FileProcessingResult fileProcessingResult) {


fileProcessingResult.getFileText is not used it is already sign smth is wrong in design.

please see my comment at #3573 (comment)

MEZk · 2017-07-13T21:13:40Z

@timurt @rnveach
Unfortunately, after discussion with @romani we came to the conclusion that this PR and changes in step 1 do not make sense at all. FileProcessingResult looks strange. It breaks the idea of TreeWalker, AST and all AST-related stuff in future should not be available for Checker. Please, see #4714 for more details.

@romani
If you are ok, please close this PR.

romani · 2017-07-14T00:22:50Z

it is better to close this PR

MEZk mentioned this pull request Jun 19, 2017

remove FileContentsHolder module as FileContents object is available for filters on TreeWalker in TreeWalkerAudit Event #3573

Closed

MEZk suggested changes Jun 19, 2017

View reviewed changes

timurt force-pushed the 3573 branch 2 times, most recently from cb726f6 to bc0c0c3 Compare June 20, 2017 05:39

MEZk suggested changes Jun 20, 2017

View reviewed changes

timurt force-pushed the 3573 branch from bc0c0c3 to 815c522 Compare June 20, 2017 17:09

timurt force-pushed the 3573 branch from 815c522 to 5b2b700 Compare June 21, 2017 06:42

timurt mentioned this pull request Jun 21, 2017

Refactor Checker to decrease class fan-out complexity #4478

Open

timurt force-pushed the 3573 branch from 5b2b700 to fd3c8f1 Compare June 21, 2017 07:50

MEZk suggested changes Jun 21, 2017

View reviewed changes

MEZk changed the title ~~Issue #3573: resovle design problem of FileContentsHolder~~ Issue #3573: Resolve design problem of FileContentsHolder (step 1) Jun 21, 2017

timurt force-pushed the 3573 branch from fd3c8f1 to f376762 Compare June 22, 2017 07:02

MEZk suggested changes Jun 22, 2017

View reviewed changes

timurt force-pushed the 3573 branch from f376762 to 8da878e Compare June 22, 2017 11:00

MEZk approved these changes Jun 22, 2017

View reviewed changes

MEZk requested a review from rnveach June 22, 2017 15:09

rnveach reviewed Jun 22, 2017

View reviewed changes

MEZk mentioned this pull request Jun 24, 2017

FileText should not extends AbstractList #3034

Closed

MEZk mentioned this pull request Jun 26, 2017

Create CheckstyleFile class to collect all information on file under processing #4527

Open

timurt force-pushed the 3573 branch from 8da878e to c1fb8f3 Compare July 9, 2017 10:03

timurt force-pushed the 3573 branch from c1fb8f3 to b97af53 Compare July 9, 2017 12:24

MEZk reviewed Jul 9, 2017

View reviewed changes

MEZk requested review from romani and rnveach and removed request for romani and rnveach July 9, 2017 17:48

timurt force-pushed the 3573 branch from b97af53 to bfe1c36 Compare July 9, 2017 18:07

timurt force-pushed the 3573 branch 2 times, most recently from e6a0c57 to b082e33 Compare July 10, 2017 09:38

timurt force-pushed the 3573 branch 6 times, most recently from 799cf44 to d8b5b75 Compare July 11, 2017 16:34

MEZk reviewed Jul 11, 2017

View reviewed changes

timurt force-pushed the 3573 branch 2 times, most recently from 562cedd to 9f264b9 Compare July 11, 2017 18:09

Issue checkstyle#3573: resovle design problem of FileContentsHolder (…

26d984a

…step 1)

timurt force-pushed the 3573 branch from 9f264b9 to 26d984a Compare July 12, 2017 06:06

romani requested changes Jul 12, 2017

View reviewed changes

romani closed this Jul 14, 2017

Issue #3573: Resolve design problem of FileContentsHolder (step 1) #4468

Issue #3573: Resolve design problem of FileContentsHolder (step 1) #4468

Conversation

timurt commented Jun 19, 2017

MEZk commented Jun 19, 2017

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

codecov-io commented Jun 20, 2017 • edited

Codecov Report

timurt commented Jun 20, 2017 • edited

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

rnveach commented Jun 20, 2017

timurt commented Jun 21, 2017

MEZk left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timurt Jun 21, 2017 • edited

Choose a reason for hiding this comment

timurt Jun 21, 2017 • edited

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

MEZk commented Jun 21, 2017 • edited

MEZk commented Jun 22, 2017

Choose a reason for hiding this comment

timurt Jun 22, 2017 • edited

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

MEZk Jun 22, 2017 • edited

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

MEZk commented Jun 22, 2017

timurt commented Jun 22, 2017

MEZk commented Jun 22, 2017

rnveach left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnveach Jun 22, 2017 • edited

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

MEZk Jun 22, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MEZk Jun 22, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MEZk Jun 22, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MEZk Jun 24, 2017 • edited

Choose a reason for hiding this comment

codecov-io commented Jun 20, 2017 •

edited

timurt commented Jun 20, 2017 •

edited

MEZk left a comment •

edited

timurt Jun 21, 2017 •

edited

timurt Jun 21, 2017 •

edited

MEZk commented Jun 21, 2017 •

edited

timurt Jun 22, 2017 •

edited

MEZk Jun 22, 2017 •

edited

rnveach Jun 22, 2017 •

edited

MEZk Jun 22, 2017 •

edited

MEZk Jun 22, 2017 •

edited

MEZk Jun 22, 2017 •

edited

MEZk Jun 24, 2017 •

edited

rnveach Jun 25, 2017 •

edited

MEZk Jun 26, 2017 •

edited

timurt Jun 23, 2017 •

edited

rnveach Jun 23, 2017 •

edited

MEZk left a comment •

edited

timurt Jul 9, 2017 •

edited