RESTWS-562 Improve Resource Definition Documentation #288

gayanW · 2017-07-14T08:18:38Z

adds swagger-core library
- uses its Java API and model classes to construct and generate the swagger json.
introudce new methods which model objects used in constructing the definitions section:
- getGETModel(Representation) : returns a object represting GET representation schema of resource
- getCREATEModel(Representation) : returns a object representing CREATE representation shema of a resource
- getUPDATEModel(Representation) : returns a object representing POST Update representation schema of a resource

ex: PersonGet, PersonCreate, PersonUpdate

adds new test cases to: SwaggerSpecificationCreatorTest.java
adds support for multiple representation types
GET schemas have support for representations: DEFAULT, REF, and FULL
CREATE schemas have support for representations: DEFAULT, and FULL
POST_UPDATE schemas have support for representations: DEFAULT
- uses default schema for the shema of responses and parameters

Swagger before: http://www.jsoneditoronline.org/?id=6ac2d0c797f5e8e4060d09b62b300b04
Swagger after: http://jsoneditoronline.org/?id=d692754b9b4543f7388572317dc10400

mention-bot · 2017-07-14T08:18:39Z

@gayanW, thanks for your PR! By analyzing the history of the files in this pull request, we identified @teleivo, @tmarzeion and @psbrandt to be potential reviewers.

dkayiwa · 2017-07-14T14:31:26Z

@gayanW did you see the travis failure?

coveralls · 2017-07-15T16:50:55Z

Coverage decreased (-5.9%) to 40.199% when pulling 9e900e6 on gayanW:restws-662 into 41d46ef on openmrs:master.

gayanW · 2017-07-15T16:55:59Z

@dkayiwa minor mistake : ) please check now

psbrandt · 2017-07-17T02:09:14Z

@bholagabbar, are you reviewing this PR?

For this to work, do developers have to implement the getGETModel, getCREATEModel and getUPDATEModel methods in their resources? I haven't taken a look at the code, but is this enforced somewhere? What happens if these methods aren't implemented?

gayanW · 2017-07-17T06:33:34Z

@psbrandt @bholagabbar

For this to work, do developers have to implement the getGETModel, getCREATEModel and getUPDATEModel methods in their resources? I haven't taken a look at the code, but is this enforced somewhere? What happens if these methods aren't implemented?

Unless introducing a new resource, it is not mandatory to implement those methods. However if not, their definitions won't appear in the swagger doc. But it will not affect the program in any other way.

I've updated almost all the resources within the rest module. Methods such as getRepresentationDescription(..) is no longer being used to generate the documentation. However I did not remove any of these existing code in case they are being accessed by other modules.

bholagabbar · 2017-07-17T08:48:23Z

@psbrandt @dkayiwa Sorry, I've been away on the Google trip. I am trying to make an attempt to yes, but reviewing 117 files changes !? There probably was a better way to break it into subtasks

bholagabbar

Have reviewed a few files. Can't possibly go through them all at once. Will get back to it in a while. Do resolve/provide justifications for the ones I have marked meanwhile

bholagabbar · 2017-07-17T08:53:30Z

omod/src/test/java/org/openmrs/module/webservices/rest/doc/SwaggerSpecificationCreatorTest.java

 		boolean limit = false, startIndex = false;

-		for (Parameter p : o.getParameters()) {
+		for (io.swagger.models.parameters.Parameter p : o.getParameters()) {


@gayanW Not good coding practice IMO. import io.swagger.models.parameters.Parameter and then use it. You've removed import org.openmrs.module.webservices.docs.swagger.Parameter so it shouldn't cause a conflict right?

I'll replace all possible fully qualified class names.

bholagabbar · 2017-07-17T08:57:42Z

omod-common/src/test/java/org/mockingbird/test/rest/resource/UnannotatedAnimalResource.java

@@ -9,6 +9,7 @@
 */


@gayanW AnimalResource? Are you using tests for the stock swagger project? The animal store one? We need to have tests with patients and other proper openmrs classes

bholagabbar · 2017-07-17T08:58:47Z

omod-common/src/test/java/org/mockingbird/test/rest/resource/AnimalClassResource_1_9.java

@@ -9,6 +9,7 @@
 */


@gayanW It seems as though all the animal tests have just been stashed here. We can't have that. Can you justify your decision? It doesn't make sense to me

They have always been there. I only implemented the missing methods. It seems that they are only used by RestServiceImplTest

@dkayiwa What do you think? Should we have these tests? On the surface, AnimalClassResource doesn't seem right inside an OpenMRS webservices module. Ideally we should be replacing these tests?

bholagabbar · 2017-07-17T09:00:07Z

...n/src/main/java/org/openmrs/module/webservices/docs/swagger/SwaggerSpecificationCreator.java

-import java.util.Map.Entry;
-import java.util.Set;
-
+import io.swagger.models.*;


@gayanW No wildcard imports. Period. Remove all of them and replace them with individual classes

bholagabbar · 2017-07-17T09:01:50Z

omod-common/pom.xml

+            <exclusions>
+                <exclusion>
+					<groupId>org.slf4j</groupId>
+					<artifactId>slf4j-api</artifactId>


Error in XML formatting. Indentation is off

bholagabbar · 2017-07-17T09:02:36Z

...penmrs/module/webservices/rest/web/v1_0/resource/openmrs2_1/CohortMembershipResource2_1.java

+			        .property("endDate", new DateProperty())
+			        .property("patientUuid", new StringProperty());
+		}
+		//FIXME missing props


@gayanW did you see the above question mark?

@dkayiwa IMO. lines marked FIXME that'll update the swagger properties will be better fix in a new separate issue.

Will be fixed in: https://issues.openmrs.org/browse/RESTWS-676

bholagabbar · 2017-07-17T09:05:00Z

...s/module/webservices/rest/web/v1_0/resource/openmrs1_9/ProviderAttributeTypeResource1_9.java

@@ -9,12 +9,14 @@
 */
 package org.openmrs.module.webservices.rest.web.v1_0.resource.openmrs1_9;

+import io.swagger.models.Model;


Is this import being used here? Get rid of all unused imports in all the files if they exist

coveralls · 2017-07-18T15:55:14Z

Coverage decreased (-5.9%) to 40.202% when pulling 5e2e234 on gayanW:restws-662 into 41d46ef on openmrs:master.

coveralls · 2017-07-18T16:05:43Z

Coverage decreased (-5.9%) to 40.202% when pulling 5e2e234 on gayanW:restws-662 into 41d46ef on openmrs:master.

gayanW · 2017-07-19T06:53:06Z

FYI: IMO reviewing all resource classes is not going to be practical. It takes a lot of effort to find right attributes, and their respective types of a resource. I had to go through ones' parent classes, supportedClass, respective tests and so on. So I think It will suffice therefore to review SwaggerSpecificationCreator and the generated json spec. And may be just go through the resource classes.

bholagabbar · 2017-07-19T08:28:37Z

@dkayiwa @psbrandt I've asked gayan to push commits as he fixes issues so it makes it easier for us to review PRs. Once we review everything and give him the go ahead, he can squash and then merge. Reviewing 280 files changes is insanity!

dkayiwa · 2017-08-07T18:02:50Z

@gayanW did you see the merge conflicts on this?

coveralls · 2017-08-08T07:19:15Z

Coverage decreased (-5.9%) to 40.261% when pulling 7b4e50d on gayanW:restws-662 into 7a39d69 on openmrs:master.

gayanW · 2017-08-08T07:19:32Z

@dkayiwa The pr was gone stale. I just rebased it.

dkayiwa · 2017-08-11T09:14:39Z

@gayanW your pull request deals with a lot of things, formatting, imports, classname changes, etc.
I would recommend a separate pull request for each of these. That way, we do not have to deal with a huge pull request whose changes are unrelated.

coveralls · 2017-08-12T07:23:56Z

Coverage decreased (-5.9%) to 40.261% when pulling 2811451 on gayanW:restws-662 into 7a39d69 on openmrs:master.

gayanW · 2017-08-12T07:30:10Z

@bholagabbar @dkayiwa
Thanks for the inputs :)

I squashed, and eliminated those formats/fix done on classes that are not directly relevant to our use case. So now we have only the changes made to the SwaggerSpecificationCreator and the resource classes.

To my understanding, it's inevitable not to update all the resources, because then it would result in a partial swagger spec.

bholagabbar

@dkayiwa I have had a cursory glance at the changes. With most of these as swagger spec definitions, I believe merging the PR and further testing it on the QA instance would be the right way to go.

bholagabbar · 2017-08-12T07:59:56Z

...ava/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_10/OrderResource1_10.java

+			        .property("previousOrder", new RefProperty("#/definitions/OrderCreate"))
+			        .property("orderReason", new RefProperty("#/definitions/ConceptCreate"));
+		}
+		//FIXME missing prop: type


@gayanW is this comment relevant?

In 1_8, orderType is marked required. But in 1_10 it seems that this property is no longer required to place a new order. For those that I thought would need extra testing, and research, I put FIXME. May we fix these in a new PR?

Mention this in the next issue you create where you will be fixing the changes you removed formatting, etc not directly relevant to this issue

dkayiwa · 2017-08-17T12:00:36Z

...ava/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_10/OrderResource1_10.java

@@ -102,6 +108,43 @@ public DelegatingResourceDescription getRepresentationDescription(Representation
 		}
 	}

+	@Override
+	public Model getCREATEModel(Representation rep) {


Did you intentionally name this as getCREATEModel instead of getCreateModel?

May be I thought that it was more readable. Anyway I could change that.

Do your mentors prefer getCREATEModel to getCreateModel?

I don't think they commented on this. If you prefer getCreateModel over getCREATEModel, I can refactor.

Can you ask them and tell me what they prefer?

Since GET, CREATE and UPDATE are actions that are being documented, captial cases indicating the action makes sense. I understand the deviation from camelCase but this makes the code more readable. I am fine with this. @dkayiwa I leave the final decision upto you

I just wanted to make sure that you are aware of this. 😊

OpenMRSFormatter.xml

dkayiwa · 2017-08-17T12:05:43Z

...a/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_8/FieldTypeResource1_8.java

+
+	@Override
+	public Model getCREATEModel(Representation rep) {
+		return super.getCREATEModel(rep); //FIXME missing props


Are the FIXME supposed to be addressed in another pull request?

Yes. As discussed with Shreyans they will be fixed in a separate issue. These will require minor to no change at all. But may need testing and discussion.

dkayiwa · 2017-08-17T12:07:40Z

...s/module/webservices/rest/web/v1_0/resource/openmrs1_8/PatientIdentifierTypeResource1_8.java

@@ -74,7 +79,7 @@ public DelegatingResourceDescription getRepresentationDescription(Representation
 			description.addProperty("validator");
 			description.addProperty("locationBehavior");
 			description.addProperty("uniquenessBehavior");
-			description.addProperty("validator");
+			description.addProperty("validator"); //FIXME duplicate


When you do plan to fix the above?

@gayanW create an issue on JIRA and paste a link here for reference

@bholagabbar should I create, before or after this is being merged?

create it now and paste the link, is there an issue with this?

https://issues.openmrs.org/browse/RESTWS-676

dkayiwa · 2017-08-17T12:12:28Z

omod/src/test/java/org/openmrs/module/webservices/rest/doc/SwaggerSpecificationCreatorTest.java

 public class SwaggerSpecificationCreatorTest extends BaseModuleWebContextSensitiveTest {

+	@Test
+	public void mainTest() {
+		String baseUrl = "localhost:8080/openmrs/ws/rest";


Does this work even when the 8080 port is used by some other process?

I just tested this with different listening and open ports. And it has no effect on where the API is exposed. It changes the baseUrl of the swagger json, and as this is a test it has no much significance.

In either case, hard coding a port number is not good practice. Make BaseUrl generic if it doesn't make a difference

The port or the baseUrl being passed in here have no actual significance. As you could see with the old tests: https://github.com/gayanW/openmrs-module-webservices.rest/blob/281145120aee4664d048b0b1cb2f6264828d2035/omod/src/test/java/org/openmrs/module/webservices/rest/doc/SwaggerSpecificationCreatorTest.java#L142

Hmm alright. I'm convinced when you tell me that it has no significance/doesn't affect the code but having something like 'http:8080/localhost' is never good practice even if it's in tests. Just assign it as 'base_url/ws/rest` or something if it doesn't have any significance.

Changed to "host/openmrs/ws/rest"
FYI: https://swagger.io/docs/specification/2-0/api-host-and-base-path/

Ok this looks good

coveralls · 2017-08-18T05:17:24Z

Coverage decreased (-5.9%) to 40.261% when pulling 270edab on gayanW:restws-662 into e12f8c7 on openmrs:master.

dkayiwa · 2017-08-23T06:18:47Z

omod-common/src/test/java/org/mockingbird/test/rest/resource/BirdResource_1_9.java

+	public Model getGETModel(Representation representation) {
+		return null;
+	}
+


I see a number of these methods simply return null. Shouldn't it be enough not to have them and assume that whichever class does not have them, null it is? Am looking at reducing the amount of code that we have to maintain.

@dkayiwa
Yeah I see. But isn't that good that we enforce them? So when creating a resource class one knows that he should document them? BTW I'm following a similar approach as it's done in old getRepresentationDescription

@dkayiwa It's your call :) ideally only a test resource should return null values. Other resources will either implement or could just inherit from its parent

In which class or interface are these methods declared?

Interface: https://github.com/gayanW/openmrs-module-webservices.rest/blob/270edab5222fdfd7b8a77adf209e6d280c1688af/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/resource/impl/DelegatingResourceHandler.java#L93-L97

Almost all resources inherit: https://github.com/gayanW/openmrs-module-webservices.rest/blob/270edab5222fdfd7b8a77adf209e6d280c1688af/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/resource/impl/BaseDelegatingResource.java#L76-L101

Yeah I just saw a minor improvement that could be done. I'll push it with the added doc.

coveralls · 2017-08-23T10:44:04Z

Coverage decreased (-5.9%) to 40.29% when pulling b9a970e on gayanW:restws-662 into e12f8c7 on openmrs:master.

gayanW · 2017-08-23T11:06:36Z

I documented the methods. see: https://github.com/gayanW/openmrs-module-webservices.rest/blob/b9a970ed4ec1c1696410e24489c3445181a7aca1/omod-common/src/main/java/org/openmrs/module/webservices/rest/web/resource/impl/DelegatingResourceHandler.java#L93-L126

And updated the issue description as well.
@dkayiwa @bholagabbar

dkayiwa · 2017-08-23T16:13:02Z

@gayanW i have tried running these changes locally and saw a few more resources show up like "address", "allergy", "answer". Was this an intentional side effect of this pull request? Or is it something else?

gayanW · 2017-08-24T06:56:41Z

@dkayiwa Swagger UI separates path operations by their tags. With the changes, subresources were tagged by their resource name instead of their parent name.

"/patient/{parent-uuid}/allergy": {
      "get": {
        "tags": [
          "allergy"
        ],
        "summary": "Fetch all non-retired allergy subresources",

So in the older spec tag is set to patient, but here it's allergy. I didn't give enough thought in to this. May be its better that we use the parent tag..?

Let me know. I'll keep changes ready.

coveralls · 2017-08-24T07:47:04Z

Coverage decreased (-5.9%) to 40.286% when pulling 07897dd on gayanW:restws-662 into e12f8c7 on openmrs:master.

dkayiwa · 2017-08-24T07:49:19Z

@gayanW can you engage the community to see what others say?

gayanW · 2017-08-24T07:51:25Z

I changed it to the old way. (FYI: only these 2 lines were changed - SwaggerSpecificationCreator.java#L602 SwaggerSpecificationCreator.java#L1001)

dkayiwa · 2017-08-24T07:52:14Z

Ok let me compile and test it out again.

gayanW · 2017-08-24T07:58:27Z

@dkayiwa
You may also have a look at the final json: https://<>/openmrs/module/webservices/rest/swagger.json

The changes will be visible once we upgrade the ui: RESTWS-560

dkayiwa · 2017-08-24T08:00:34Z

So how will i see the changes before RESTWS-560 is done?

gayanW · 2017-08-24T08:03:19Z

@dkayiwa Some changes are still visible with the old ui. But if you want to see all the changes, you may try this out: http://editor.swagger.io/

And paste the content of the swagger.json there.

dkayiwa · 2017-08-24T08:04:21Z

How long would it take to finish the other ticket?

gayanW · 2017-08-24T08:06:37Z

@dkayiwa It is better to work on it once this is finished.
BTW it won't take much time to do the ui upgrade. I've already tested locally.

dkayiwa · 2017-08-24T10:09:45Z

@gayanW i have just tested again and noticed that the q parameter is not highlighted as required, with your changes. Was that intentional? https://qa-refapp.openmrs.org/openmrs/module/webservices/rest/apiDocs.htm#!/order/getAllOrders

gayanW · 2017-08-24T11:24:20Z

@dkayiwa
Are you sure it is not marked required for GET /order?
http://localhost:8080/openmrs/module/webservices/rest/apiDocs.htm#!/order/getAllOrders
I ran locally and it is marked required.

q param is not marked required for resources that supports fetchAll.
/concept Fetch all non-retired concept resources or perform search
http://localhost:8080/openmrs/module/webservices/rest/apiDocs.htm#!/concept/getAllConcepts

If there's more than one query param, then it is not marked required either.
http://localhost:8080/openmrs/module/webservices/rest/apiDocs.htm#!/encounter/getAllEncounters

- adds swagger-core library * uses its Java API and model classes to construct and generate the swagger json. - introudce new methods which model objects used in constructing the definitions section: * getGETModel(Representation) : returns a object represting GET representation schema of resource * getCREATEModel(Representation) : returns a object representing CREATE representation shema of a resource * getUPDATEModel(Representation) : returns a object representing POST Update representation schema of a resource ex: PersonGet, PersonCreate, PersonUpdate - adds new test cases to: SwaggerSpecificationCreatorTest.java - adds support for multiple representation types GET schemas have support for representations: DEFAULT, REF, and FULL CREATE schemas have support for representations: DEFAULT, and FULL POST_UPDATE schemas have support for representations: DEFAULT * uses default schema for the shema of responses and parameters

dkayiwa · 2017-08-24T12:02:28Z

@bholagabbar what behavior do you get in regards to the above?

coveralls · 2017-08-24T12:18:33Z

Coverage decreased (-5.9%) to 40.286% when pulling c739c17 on gayanW:restws-662 into e12f8c7 on openmrs:master.

dkayiwa · 2017-08-24T12:47:04Z

...ava/org/openmrs/module/webservices/rest/web/v1_0/resource/openmrs1_10/OrderResource1_10.java

+		        .property("accessionNumber", new StringProperty())
+		        .property("dateActivated", new DateProperty())
+		        .property("scheduledDate", new DateProperty())
+		        .property("patient", new StringProperty().example("uuid"))


Where are these examples displayed?

http://localhost:8080/openmrs/module/webservices/rest/apiDocs.htm#!/order/createOrder
See in the 'Example Value' section.

{
"encounter": "uuid",
"action": "NEW",
"accessionNumber": "string",
"dateActivated": "2017-08-24",
"patient": "uuid",
"concept": "uuid",
...
}

dkayiwa · 2017-08-24T15:16:52Z

...penmrs/module/webservices/rest/web/v1_0/resource/openmrs1_11/DrugIngredientResource1_11.java

+	public Model getGETModel(Representation representation) {
+		return null;
+	}
+


What is the difference between a resource like this which returns all nulls and those that do not? Or was it simply a lack of time to implement?

No it's not due to lack of time. I had enough time :) I've been tracking undocumented resources by validating the swagger. Some how they were missed. I'll check and fix those.

And i found a good number of them.

dkayiwa · 2017-08-24T21:07:52Z

...nmrs/module/webservices/rest/web/v1_0/resource/openmrs1_10/TestOrderSubclassHandler1_10.java

+		return null;
+	}
+
+	@Override


Were these intentionally left null?

Do we need to document handlers? I don't think they appear either as a resource or a property. If needed let me know.

Then in that case we should not have these methods in them.

However they implements getRepresentationDescription, getCreatableProperties, and getUpdatableProperties. Is handler considered a resource or can they be a property of some other resource?

I would not consider it as a resource. But putting this aside, can you ensure that all other non search handlers that should not just return null, are doing what they should?

dkayiwa · 2017-08-24T21:08:31Z

...penmrs/module/webservices/rest/web/v1_0/resource/openmrs1_12/OrderSetMemberResource1_12.java

@@ -99,6 +100,16 @@ public DelegatingResourceDescription getUpdatableProperties() {
 	}

 	@Override
+	public Model getGETModel(Representation representation) {
+		return null;
+	}


Were these intentionally left null? And many more others.

gayanW

SwaggerSpecificationCreator#mainTest() does not detect resources from all versions. This is not really an issue. But it will be useful, if it could detect all, so one can generate the spec just from running this test, instead of having to restart the server.

Can it be done?

coveralls · 2017-08-25T14:32:00Z

Coverage decreased (-6.2%) to 40.023% when pulling 4c74be2 on gayanW:restws-662 into e12f8c7 on openmrs:master.

gayanW · 2017-08-25T14:34:53Z

@dkayiwa
ModuleResource1_8, OrderableResource1_10, CustomDatatypeResource1_9, ConceptSearchResource1_9, and CustomDatatypeHandlerResource1_9 returns null for getCREATEModel(rep) because they are documented as not creatable. If you check these classes, you'll see that none of them implement getCreatableProperties() and throws ResourceDoesNotSupportOperationException when invoked.

Test resource classes also returns null, as they are not needed.

Except these, all other classes are documented.

dkayiwa · 2017-08-25T14:50:40Z

Ok cool.
Am merging this earlier than later such that you get un blocked.
After merging, i will take a second look and if any more work is needed, you will do that in a new pull request. Otherwise thanks for the work so far done. 😊

gayanW force-pushed the restws-662 branch from e6cef7c to 9e900e6 Compare July 15, 2017 16:38

bholagabbar reviewed Jul 17, 2017

View reviewed changes

gayanW force-pushed the restws-662 branch from 5e2e234 to 7b4e50d Compare August 8, 2017 07:07

gayanW force-pushed the restws-662 branch from 7b4e50d to 2811451 Compare August 12, 2017 07:11

bholagabbar reviewed Aug 12, 2017

View reviewed changes

dkayiwa reviewed Aug 17, 2017

View reviewed changes

OpenMRSFormatter.xml Show resolved Hide resolved

dkayiwa reviewed Aug 17, 2017

View reviewed changes

gayanW force-pushed the restws-662 branch from 2811451 to 270edab Compare August 18, 2017 05:05

dkayiwa reviewed Aug 23, 2017

View reviewed changes

gayanW force-pushed the restws-662 branch from b9a970e to 07897dd Compare August 24, 2017 07:35

gayanW force-pushed the restws-662 branch from 07897dd to c739c17 Compare August 24, 2017 12:01

dkayiwa reviewed Aug 24, 2017

View reviewed changes

gayanW commented Aug 25, 2017

View reviewed changes

Fixup, document undocumented methods

4c74be2

dkayiwa merged commit 639ad02 into openmrs:master Aug 25, 2017

RESTWS-562 Improve Resource Definition Documentation #288

RESTWS-562 Improve Resource Definition Documentation #288

Conversation

gayanW commented Jul 14, 2017

mention-bot commented Jul 14, 2017

dkayiwa commented Jul 14, 2017

coveralls commented Jul 15, 2017

gayanW commented Jul 15, 2017

psbrandt commented Jul 17, 2017

gayanW commented Jul 17, 2017 • edited Loading

bholagabbar commented Jul 17, 2017

bholagabbar 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Jul 18, 2017

coveralls commented Jul 18, 2017

gayanW commented Jul 19, 2017

bholagabbar commented Jul 19, 2017

dkayiwa commented Aug 7, 2017 • edited Loading

coveralls commented Aug 8, 2017

gayanW commented Aug 8, 2017

dkayiwa commented Aug 11, 2017

coveralls commented Aug 12, 2017

gayanW commented Aug 12, 2017

bholagabbar 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

bholagabbar Aug 17, 2017 • 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

gayanW Aug 17, 2017 • 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

coveralls commented Aug 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gayanW Aug 23, 2017 • edited Loading

Choose a reason for hiding this comment

dkayiwa Aug 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Aug 23, 2017

gayanW commented Aug 23, 2017

dkayiwa commented Aug 23, 2017

gayanW commented Aug 24, 2017

coveralls commented Aug 24, 2017

dkayiwa commented Aug 24, 2017

gayanW commented Jul 17, 2017 •

edited

Loading

dkayiwa commented Aug 7, 2017 •

edited

Loading

bholagabbar Aug 17, 2017 •

edited

Loading

gayanW Aug 17, 2017 •

edited

Loading

gayanW Aug 23, 2017 •

edited

Loading

dkayiwa Aug 23, 2017 •

edited

Loading

gayanW commented Aug 24, 2017 •

edited

Loading

gayanW commented Aug 24, 2017 •

edited

Loading