Issue #464: Check for trailing commas on enums #709

kariem · 2018-07-26T12:11:18Z

Addresses #464, supersedes #465 and #474.

This Check assures that the last enum constant definition in a multi-line enum ends in a comma:

enum E1 {
    ONE,
    TWO,
    THREE,
}

Usage examples are in the Javadoc of EnumTrailingCommaCheck. This check follows the concept of ArrayTrailingComma, hence the similar name.

It also checks that the "trailing" comma after the last enum constant definition is not followed by a semicolon, because this would miss the point of the check: clean diff and easy manipulation.

coveralls · 2018-07-26T12:18:13Z

Coverage increased (+0.006%) to 98.777% when pulling 9d9f851 on kariem:464-enum-trailing-comma-and-semicolon into 67d2e19 on sevntu-checkstyle:master.

kariem · 2018-07-26T12:25:17Z

Please let me know what has to be done to fix the builds. I will try to get around this within the next few days.

Please also let me know what needs to be done in addition to merge this pull request.

rnveach · 2018-07-26T14:34:46Z

The following are the CI failures.

All sevntu checks should be used in contribution

Check must be added to https://raw.githubusercontent.com/checkstyle/contribution/master/checkstyle-tester/checks-sevntu-error.xml .

checkstyle-regression
Unable to instantiate 'com.github.sevntu.checkstyle.checks.coding.EnumTrailingCommaAndSemicolonCheck' class
wercker
org.eclipse.aether.resolution.ArtifactResolutionException: Failure to find com.github.sevntu-checkstyle:sevntu-checkstyle-maven-plugin:jar:1.30.0

I will have to see what is wrong.

Checkstyle regression will require changes in main checkstyle project based on violations from this check.
@romani Confirm you are ok with these type of changes in main project.

romani · 2018-07-26T16:13:16Z

@kariem ,

Checkstyle regression will require changes in main checkstyle project based on violations from this check.

Please provide update to our code in separate commit, it better to make it first commit in this PR, as it unlikely to change. Such commit culd be merged even before main fix to ease merge of new Check.

romani · 2018-07-26T17:26:02Z

Hmm, a lot time passes from original discussion, originally reported at checkstyle/checkstyle#3161. We are not in moratorium any more, so if @kariem ok it is better to contribute to main project directly. But we can finish contribution to sevntu first, whatever is easier.

@rnveach , please review main issue and approve it.

rnveach · 2018-07-27T02:56:03Z

checkstyle-regression
Unable to instantiate 'com.github.sevntu.checkstyle.checks.coding.EnumTrailingCommaAndSemicolonCheck' class

This was fixed. CI now shows results of regresion. Main checkstyle project has 27 violations from this new check.
See https://travis-ci.org/sevntu-checkstyle/sevntu.checkstyle/jobs/408476345#L1063

romani · 2018-07-30T09:37:53Z

Wercker failure is strange, I cleaned cache and relaunched build.

rnveach · 2018-07-30T11:29:58Z

wercker passed, so CI validation is working correctly now.

kariem · 2018-07-30T16:14:12Z

@romani Sorry, I do not understand what needs to be done. If I have to change the checkstyle code, I will have to do this in another PR on checkstyle/checkstyle, right?

What's your recommendation? I think you guys can better assess what is the best way forward.

romani · 2018-07-30T16:30:54Z

I suggest to finish merge to sevntu, as a lot already done and we are close to merge state.
Moving to main repo could cost extra changes for quality, a quality level in main repo significantly higher.

It is better to merge, this PR sooner.

rnveach · 2018-07-30T16:38:53Z

@romani Then it is your call on what to do in this PR. CI will never be green here without changes to main repo and external file.

romani · 2018-07-31T08:47:25Z

Let's merge new Check to sevntu first.

Failure in checkstyle-regression
https://travis-ci.org/sevntu-checkstyle/sevntu.checkstyle/jobs/408476345#L1028 should be fixed in code of main repo. @kariem , please send PR to main repo with fixes for missed comma. We we quickly merge it .

Failure "All sevntu checks should be used in contribution"

Please send PR to https://github.com/checkstyle/contribution/blob/master/checkstyle-tester/checks-sevntu-error.xml
We will merge it right after release of your new Check. It is ok to keep this CI point to be failed, it is not a blocked for merge.

kariem · 2018-07-31T23:49:50Z

please send PR to main repo with fixes for missed comma. We we quickly merge it.

I have created a pull request in checkstyle/checkstyle (referenced above). However, there seem to be conflicts with other checks. Is it necessary to enable all checks? I think it's quite obvious that some checks do not play well with others.

Please send PR to https://github.com/checkstyle/contribution/blob/master/checkstyle-tester/checks-sevntu-error.xml

Does that mean that I should add EnumTrailingCommaAndSemicolonCheck to this configuration?

romani · 2018-08-01T09:07:33Z

I think it's quite obvious that some checks do not play well with others.

I didn't predicted a conflict , so it is not obvious. Need to be warned in documentation as minimum.

Does that mean that I should add EnumTrailingCommaAndSemicolonCheck to this configuration?

Yes, please do, sorry for confusion.

kariem · 2018-08-01T09:44:28Z

I think it's quite obvious that some checks do not play well with others.

I didn't predicted a conflict , so it is not obvious.

I was talking about the general implication of requirements for code formatting. It is obvious to me that some checks have conflicting requirements. In this special case, I did not know that there were conflicts until I have seen them. However, I assumed that you were aware of checks that are already in place in CI.

Need to be warned in documentation as minimum.

Done: 5917f4f. Is this sufficient for you?

kariem · 2018-08-01T09:44:38Z

I will add to the checkstyle/contribution/checkstyle-tester/checks-sevntu-error.xml as soon as we have agreed on a solution to the conflicting checks. I hope this also makes sense to you.

romani · 2018-08-01T16:48:13Z

Is this sufficient for you?

Good. Please squash commits in one.

conflicting checks. I hope this also makes sense to you.

Please add it with ignore severity. Looks like Check is too conflicting now.

rnveach · 2018-08-01T20:32:26Z

@kariem To disable regression for your new check, please add it to the ignore list at:
https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/master/sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CheckstyleRegressionTest.java#L37

rnveach · 2018-08-01T20:34:39Z

I will add to the checkstyle/contribution/checkstyle-tester/checks-sevntu-error.xml as soon as we have agreed on a solution to the conflicting checks.

The check can still be added, as is, to this configuration. The configuration already sets severity to ignore for all checks. See https://github.com/checkstyle/contribution/blob/789055b817de65290d31c125f5b7cb256679197f/checkstyle-tester/checks-sevntu-error.xml#L15 .

kariem · 2018-08-02T08:31:16Z

I have now disabled the check for regression as instructed by rnveach and squashed all commits into one.

I have created a separate issue and PR on checkstyle/contribution for inclusion of EnumTrailingCommaAndSemicolonCheck.

@rnveach @romani Please let me know, if there is anything else I can do now.

romani · 2018-08-02T15:32:24Z

I need some time to review, I am afk till Tuesday .

Side note is that logic should be "similar" to http://checkstyle.sourceforge.net/config_coding.html#ArrayTrailingComma

Looks like trailing ; is optional if nothing else except elements defined.
I am not sure for now, but smth tell me there should be special processing of enums with other members (methods).

romani

items to improve:

romani · 2018-10-16T13:28:35Z

eclipsecs-sevntu-plugin/src/com/github/sevntu/checkstyle/checks/coding/checkstyle-metadata.xml

+      <description>%EnumTrailingCommaCheck.desc</description>
+
+      <message-key key="enum.trailing.comma"/>
+      <message-key key="enum.trailing.comma.semi"/>


please remove this line.

Done: 5ee1a62

romani · 2018-10-16T13:33:01Z

...-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaCheck.java

+        }
+    }
+
+    private boolean isIllegalTokenAfterComma(DetailAST lastComma, DetailAST nextAst) {


please explain why this method is not isSmthAfterCommaOnSameLine with no special processing of SEMI.
I think it should be checking for any sibling on same line.

According to the Java Language Specification on Enum Constants the only allowed symbols are

Other enum constants, separated by a comma

Enum body declarations, starting with a semicolon

If this method does not allow any sibling on the same line, this would also include traditional comments (starting with TokenType.BLOCK_COMMENT_BEGIN) and end-of-line comments (TokenType.SINGLE_LINE_COMMENT). Both, however, should be allowed. Am I misunderstanding something?

your Check is not comments aware, so in AST there will be no comments at all.
example of how comments could appear in AST for Check - https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/indentation/CommentsIndentationCheck.java#L104

so if you are not comment aware check - I see no reason in extra validation of tokens.
If you want your Check to be aware of comments , you can do this, and describe in documentation of Check that comments are ignored (so it is the same as working on AST without comments).

please make a UT case in input file with comment to make it clear that you do not violate such lines.

If this resolve an issue, I would appreciate your feedback on how to improve our documentation on comments in AST.

@romani Just checked the implementation again. Good to know that we do not need to check for comments, so the explicit check on symbols seems to be redundant.

However, after looking at this again, I still think the current version is very clear on what is done: we have two problem cases:

The last comma is followed by an enum constant: this means that there is no more comma after that enum constant, because we have already checked for the last comma

The last comma is followed by a semicolon on the same line.

This is the implementation

private boolean isIllegalTokenAfterComma(DetailAST lastComma, DetailAST nextAst) { boolean illegalToken = false; final int nextAstType = nextAst.getType(); if (nextAstType == TokenTypes.SEMI) { if (nextAst.getLineNo() == lastComma.getLineNo()) { // semi on the same line as last comma illegalToken = true; } } else if (nextAstType == TokenTypes.ENUM_CONSTANT_DEF) { // next enum constant on the same line after comma illegalToken = true; } return illegalToken; }

I have now tried different options and could not find a clearer solution, so I do not really understand what you are asking for when you do not want the "extra validatoin" of tokens. We will always have to check for the semicolon or the right curly bracket to see whether we are at the end of the enum constant block.

Also, this method could be a lot less verbose so I have just updated it. See be8be03

private boolean isIllegalTokenAfterComma(DetailAST lastComma, DetailAST nextAst) { final int nextAstType = nextAst.getType(); // semi on the same line as last comma, or followed by enum constant return (nextAstType == TokenTypes.SEMI && nextAst.getLineNo() == lastComma.getLineNo()) || nextAstType == TokenTypes.ENUM_CONSTANT_DEF; }

Does that work for you?

The last comma is followed by an enum constant: this means that there is no more comma after that enum constant, because we have already checked for the last comma
The last comma is followed by a semicolon on the same line.

Fact that I do not understand is why we care about "last comma" ? all we should care is last enum element and symbol that is following it. If following symbol/element is not comma - it is violation. If after comma there is smth on same line - it is violation.
In other words - recheck two elements of AST after last enum constant.

I have rewritten that whole block. It's now similar to the ArrayTrailingCommaCheck, with some additional comments:

enumConstEnd can either be SEMI or RCURLY.

// Only check, if block is multi-line and there are more than one enum constants if (enumConstLeft.getLineNo() != enumConstEnd.getLineNo() && enumConstBlock.getChildCount(TokenTypes.ENUM_CONSTANT_DEF) > 1) { final DetailAST lastSibling = enumConstEnd.getPreviousSibling(); if (lastSibling.getType() != TokenTypes.COMMA) { // constant definition does not end in a comma log(lastSibling, MSG_KEY); } else if (enumConstEnd.getType() == TokenTypes.SEMI && enumConstEnd.getLineNo() == lastSibling.getLineNo()) { // constant definition ends in a comma, but is followed by semi on same line log(enumConstEnd, MSG_KEY); } }

With this, we can remove the other helper methods getLastComma and isIllegalTokenAfterComma.

Does that work for you?

@romani I haven't directly answered the question, but I think the current solution is better than I had previously suggested. Let me quickly answer the question

Fact that I do not understand is why we care about "last comma"?

We only care about the "last comma". This is the point of the check. Hence the name EnumTrailingCommaCheck. Intermediary commas are required as per language specification. Only the last, the trailing comma, is relevant for this check.

sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CheckstyleRegressionTest.java

romani · 2018-10-16T13:37:22Z

sevntu-checks/src/test/java/com/github/sevntu/checkstyle/internal/CheckstyleRegressionTest.java

-            .asList("ReturnCountExtendedCheck");
+            .asList(
+                    "EnumTrailingCommaCheck",
+                    "ReturnCountExtendedCheck"


please add comment above "// initial ",
we will update it ones your PR is merged.
@rnveach , do you remember a reason ?

@romani The reason was probably because it wasn't enabled in main repo as this is regression script.
See checkstyle/checkstyle#3496 where it was enforced and checkstyle/checkstyle#3628 where it was first added. The last issue is a direct link to this script, so the check was never enabled at the time the regression was created.
This line can probably be removed.

See my comment on the above review note: I have only added the comment to the line for EnumTrailingCommaCheck. I don't think touching the code regarding ReturnCountExtendedCheck is in the scope of this PR.

ok, look ok now , I will update reason for second issue , ones we merge you.

rnveach · 2018-10-16T13:42:32Z

CI is failing.
https://travis-ci.org/sevntu-checkstyle/sevntu.checkstyle/jobs/442062494#L559

[ERROR] src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaCheck.java:[134,5] (javadoc) JavadocMethod: Missing a Javadoc comment.
[ERROR] src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaCheck.java:[134,5] (coding) ReturnCount: Return count is 2 (max allowed for non-void methods/lambdas is 1).

kariem · 2018-10-17T09:15:23Z

CI is failing. https://travis-ci.org/sevntu-checkstyle/sevntu.checkstyle/jobs/442062494#L559

Resolved in 26d7bf9 and bc2730a.

kariem · 2018-10-17T09:20:54Z

Squashed all changes and pushed again: 4a30256

Please let me know what you think needs to be done in addition to merge this pull request.

It would be very helpful, if you could write all up in one go. The back and forth is very time consuming for all parties involved.

romani · 2018-10-17T19:28:33Z

It would be very helpful, if you could write all up in one go.

Sorry, it is all my bad, I agree, but I do review of all PRs and requests and issues, it is hard fro me to spend whole my time only on one PR and make it merged with minimal amount of interactions.
In most of the time, I just do not see all problems till the very end. Sorry my bad, I hope to improve my skill to see all conner cases in one review.
I appreciate your persistence to make this PR be merged.

kariem · 2018-10-21T11:32:48Z

Squashed all changes and pushed again: 1b87398

Please let me know what you think needs to be done in addition to merge this pull request.

rnveach · 2018-10-21T15:45:59Z

@kariem there is still a checkstyle violation, be sure to run mvn verify test before pushing to the server

[INFO] There is 1 error reported by Checkstyle 8.12 with sevntu-checks.xml ruleset.
[ERROR] src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaCheck.java:[147,17] (extension) LogicConditionNeedOptimization: Condition with || at line 147 position 16 need optimization. All method calls are advised to move to end of logic expression.

kariem · 2018-10-21T16:15:08Z

there is still a checkstyle violation, be sure to run mvn verify test before pushing to the server

@rnveach I did run this and did not get any errors. I re-ran it again after an mvn clean and still did not get any errors.

[ERROR] src/main/java/com/github/sevntu/checkstyle/checks/coding/EnumTrailingCommaCheck.java:[147,17] (extension) LogicConditionNeedOptimization: Condition with || at line 147 position 16 need optimization. All method calls are advised to move to end of logic expression.

Thank you for the information. This relates to this line:

// semi on the same line as last comma, or followed by enum constant
return (nextAstType == TokenTypes.SEMI && nextAst.getLineNo() == lastComma.getLineNo())
        || nextAstType == TokenTypes.ENUM_CONSTANT_DEF;

As I cannot reproduce this and do not want to push changes until it works, could you tell me, how I can correct this line? Should I blow it up again as we had this as indicated in #709 (comment) ?

Thank you for your help.

rnveach · 2018-10-22T02:24:44Z

Sorry, for sevntu to check itself the command is mvn -e verify -Pno-validations,selftesting.

The violation just wants you to swap the contents between the ||. So instead of a || b, it wants b || a.

kariem · 2018-10-22T16:45:47Z

Thank you for the clarification! Just updated this part.

…

On Mon, Oct 22, 2018 at 4:38 AM rnveach ***@***.***> wrote: Sorry, for sevntu to check itself the command is mvn -e verify -Pno-validations,selftesting. The violation just wants you to swap the contents between the ||. So instead of a || b, it wants b || a. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#709 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAcCPEyNRc4HK6J829ZPoc3QBfkmZLkXks5unS-5gaJpZM4VhtF6> .

romani

reply to old discussion - dabe711#r227181567

Please make in input cases like:

One, Two // is it a violation ?
, Three, Four
;

And

One, Two
;

I just want to be sure we are know what we are validating, and clear define this in description for users

extra minor item to improve:

romani · 2018-10-23T00:22:33Z

...c/test/resources/com/github/sevntu/checkstyle/checks/coding/InputEnumTrailingCommaCheck.java

+    enum E5 {
+        ONE,
+        TWO,
+        THREE,


please make a test case with trailing comments (// some comment) after "ONE" and after "TREE" to make it clear that Check do not see comments at all.

@romani

Added a separate section at the end for both, traditional and end-of-line comments.

// check ignores comments enum E9_1 { ONE, // this is ok TWO, THREE, } enum E9_2 { ONE, TWO /* this is also ok */ , THREE, ; }

@romani I missed to address the first part of this request.

Changes:

Separate section with your exampe, slightly adapted
// all ok enum E_1 { ONE, TWO , THREE, FOUR, }
The trailing comma after FOUR is important here

Included another check on the missing enum, if followed by a semicolon on the next line
enum E2_2 { ONE, TWO, THREE // violation ; }

I have also added more information to the // violation comments to explain the respective reasons.

kariem · 2018-10-23T15:52:48Z

@romani From my perspective this is done. From what I can see, all the side-conversations that have not been closed or resolved are either resolved or outdated and redundant.

If there is anything else for me to do here, please write one comment with everything that needs to be done. This way I can do everything you want and have this pull request closed. The current process is very annoying and time consuming.

romani · 2018-10-25T03:46:20Z

@kariem , sorry that it takes so long but idea of Check somewhere is broken that is wh awe sendup with long discussion.
Lets close this PR and create new PR to start discussion from scratch.

I still see inconsistency in behavior and what Check try to address.
at javadoc

Checks if enum constant contains an optional trailing comma.

Behavior:


   enum Type {
       ALPHA,
       BETA,
       GAMMA,   // changed due to the ','
       DELTA    // new line
  }

could be done as
   enum Type {
       ALPHA,
       BETA,
       GAMMA   // no changes
       , DELTA    // new line
  }


     enum E_1 {
         ONE, TWO
         , THREE, FOUR,
     }

We need to clearly define what we are validating.

Se you in new PR.

kariem · 2018-10-25T05:42:29Z

@romani Please describe what the inconsistency is for you and I will try to address this. The idea of the check follows ArrayTrailingComma. If you do not want what is done in ArrayTrailingComma, we have a different discussion.

I do not think a new PR helps here, as you might have noticed, this is PR number 4 and I have addressed all your requests in the previous PRs and in this PR.

See #722 for a follow-up.

romani · 2018-10-25T12:34:45Z

Javadoc first sentence just tell about training comma, but in fact Check care only about comma after last element. This need to be fixed in javadoc if this is intentional.

This check does not demand each element of enum to be on separate line. If that is intentional we just need to make it clear in javadoc, to make it clear that this Check address only particular case.

kariem · 2018-10-25T13:22:54Z

@romani

Javadoc first sentence just tell about training comma, but in fact Check care only about comma after last element.

Trailing comma is the comma after the last element. Either there is a trailing comma, or there is none. This check is only about trailing commas. Please see similar rules in JavaScript, TSLint, or the often linked ArrayTrailingComma in Checkstyle (also referred to in the javadoc).

This check does not demand each element of enum to be on separate line. If that is intentional we just need to make it clear in javadoc, to make it clear that this Check address only particular case.

Of course, this does not demand anything else. This also ignores single-line enum constant definitions, because there is no benefit. This does not check comments, as they are not really relevant for what we are doing.

Please also go through your discussion around the check for trailing commas in arrays in checkstyle/checkstyle#1509

I have tried to use terms commonly known in Checkstyle and also known in other closely related languages. I did not invent anything new here and for me the description is very clear. If you think there is a problem with the terms used, please suggest an improvement. If you think something is not clear enough, please suggest a clarification.

romani · 2018-10-25T14:20:50Z

Sorry that I ma demanding, but I do this for a reason. There are hundreds of Checks and their goals a not very clear that result in many bugs reported on Checks and had to keep project under control.

Please see my comments in new PR.

kariem mentioned this pull request Jul 26, 2018

Issue #464: Check for trailing commas on enums #474

Closed

rnveach mentioned this pull request Jul 27, 2018

config: upgraded sevntu-checkstyle-maven-plugin to 1.30.0 checkstyle/checkstyle#6050

Merged

rnveach mentioned this pull request Jul 27, 2018

config: upgraded sevntu-checkstyle-maven-plugin to 1.30.0 checkstyle/contribution#331

Merged

kariem mentioned this pull request Jul 31, 2018

Issue #3161: Format for trailing comma check in enums checkstyle/checkstyle#6054

Merged

kariem force-pushed the 464-enum-trailing-comma-and-semicolon branch from b54be53 to 5917f4f Compare August 1, 2018 09:43

kariem force-pushed the 464-enum-trailing-comma-and-semicolon branch 2 times, most recently from b43332b to b2b1aed Compare August 2, 2018 08:13

This was referenced Aug 2, 2018

Include EnumTrailingCommaAndSemicolonCheck checkstyle/contribution#332

Open

Issue #332: Include EnumTrailingCommaAndSemicolonCheck checkstyle/contribution#333

Closed

romani requested changes Oct 16, 2018

View reviewed changes

kariem force-pushed the 464-enum-trailing-comma-and-semicolon branch from bc2730a to 4a30256 Compare October 17, 2018 09:16

kariem force-pushed the 464-enum-trailing-comma-and-semicolon branch 2 times, most recently from f5906c1 to 1b87398 Compare October 21, 2018 11:32

kariem force-pushed the 464-enum-trailing-comma-and-semicolon branch from 1b87398 to dabe711 Compare October 22, 2018 09:35

romani requested changes Oct 23, 2018

View reviewed changes

kariem force-pushed the 464-enum-trailing-comma-and-semicolon branch from dabe711 to 3a827f1 Compare October 23, 2018 15:41

kariem force-pushed the 464-enum-trailing-comma-and-semicolon branch from 3a827f1 to 57428dc Compare October 23, 2018 15:53

Issue sevntu-checkstyle#464: Check for trailing commas on enums

b86c563

kariem force-pushed the 464-enum-trailing-comma-and-semicolon branch from 57428dc to b86c563 Compare October 23, 2018 15:59

romani closed this Oct 25, 2018

kariem mentioned this pull request Oct 25, 2018

Issue #464: Check for trailing commas on enums #722

Closed

romani mentioned this pull request May 19, 2019

New Check: UnnecessarySemicolon checkstyle/checkstyle#6752

Closed

kariem mentioned this pull request Oct 18, 2022

New check: Trailing comma in enum similar to array initializer #464

Open

Zopsss mentioned this pull request Mar 26, 2024

minor: corrected leading asterisks alignment in javadoc #1035

Merged

Issue #464: Check for trailing commas on enums #709

Issue #464: Check for trailing commas on enums #709

Conversation

kariem commented Jul 26, 2018 • edited Loading

coveralls commented Jul 26, 2018 • edited Loading

kariem commented Jul 26, 2018

rnveach commented Jul 26, 2018

romani commented Jul 26, 2018

romani commented Jul 26, 2018

rnveach commented Jul 27, 2018

romani commented Jul 30, 2018

rnveach commented Jul 30, 2018

kariem commented Jul 30, 2018

romani commented Jul 30, 2018

rnveach commented Jul 30, 2018 • edited Loading

romani commented Jul 31, 2018

kariem commented Jul 31, 2018

romani commented Aug 1, 2018

kariem commented Aug 1, 2018

kariem commented Aug 1, 2018

romani commented Aug 1, 2018

rnveach commented Aug 1, 2018

rnveach commented Aug 1, 2018

kariem commented Aug 2, 2018

romani commented Aug 2, 2018 • edited Loading

romani 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

romani Oct 23, 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

rnveach Oct 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rnveach commented Oct 16, 2018

kariem commented Oct 17, 2018

kariem commented Oct 17, 2018

romani commented Oct 17, 2018

kariem commented Oct 21, 2018

rnveach commented Oct 21, 2018

kariem commented Oct 21, 2018

rnveach commented Oct 22, 2018

kariem commented Oct 22, 2018 via email

romani left a comment • 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

kariem commented Oct 23, 2018

romani commented Oct 25, 2018 • edited Loading

kariem commented Oct 25, 2018

romani commented Oct 25, 2018

kariem commented Oct 25, 2018

romani commented Oct 25, 2018

kariem commented Jul 26, 2018 •

edited

Loading

coveralls commented Jul 26, 2018 •

edited

Loading

rnveach commented Jul 30, 2018 •

edited

Loading

romani commented Aug 2, 2018 •

edited

Loading

romani Oct 23, 2018 •

edited

Loading

rnveach Oct 16, 2018 •

edited

Loading

romani left a comment •

edited

Loading

romani commented Oct 25, 2018 •

edited

Loading