Allow specifying the runOrder property when creating CodePipeline Actions #776

skinny85 · 2018-09-25T23:06:29Z

Also create a new addToPipeline method on LambdaRef,
which was missed when doing these for Code* services.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.

skinny85 · 2018-09-26T00:19:55Z

Interesting. I believe this changed might have uncovered a bug in JSII:

[ERROR] COMPILATION ERROR : 
[INFO] -------------------------------------------------------------
[ERROR] /tmp/jsii-pacmak-code9apQBY/src/main/java/software/amazon/awscdk/services/s3/PipelineSourceActionProps.java:[37,34] variable _runOrder is already defined in class software.amazon.awscdk.services.s3.PipelineSourceActionProps.Builder
[ERROR] /tmp/jsii-pacmak-code9apQBY/src/main/java/software/amazon/awscdk/services/s3/PipelineSourceActionProps.java:[101,24] method withRunOrder(java.lang.Number) is already defined in class software.amazon.awscdk.services.s3.PipelineSourceActionProps.Builder
[ERROR] /tmp/jsii-pacmak-code9apQBY/src/main/java/software/amazon/awscdk/services/s3/PipelineSourceActionProps.java:[122,42] variable $runOrder is already defined in class <anonymous software.amazon.awscdk.services.s3.PipelineSourceActionProps$Builder$1>
[ERROR] /tmp/jsii-pacmak-code9apQBY/src/main/java/software/amazon/awscdk/services/s3/PipelineSourceActionProps.java:[185,41] method getRunOrder() is already defined in class <anonymous software.amazon.awscdk.services.s3.PipelineSourceActionProps$Builder$1>
[ERROR] /tmp/jsii-pacmak-code9apQBY/src/main/java/software/amazon/awscdk/services/s3/PipelineSourceActionProps.java:[190,29] method setRunOrder(java.lang.Number) is already defined in class <anonymous software.amazon.awscdk.services.s3.PipelineSourceActionProps$Builder$1>
[INFO] 5 errors 
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 3.309 s
[INFO] Finished at: 2018-09-25T23:21:40+00:00
[INFO] Final Memory: 17M/291M
[INFO] ------------------------------------------------------------------------

Could it be possible the reason is that the hierarchy of interfaces is as follows:

codepipeline.CommonActionConstructProps extends codepipeline.CommonActionProps
s3.CommonPipelineSourceActionProps extends codepipeline.CommonActionProps
s3.PipelineSourceActionProps extends s3.CommonPipelineSourceActionProps and codepipeline.CommonActionConstructProps

, and codepipeline.CommonActionProps wounds up being in the inheritance hierarchy twice?

eladb · 2018-09-26T08:32:06Z

packages/@aws-cdk/aws-cloudformation/lib/pipeline-actions.ts

@@ -48,6 +48,7 @@ export abstract class PipelineCloudFormationAction extends codepipeline.DeployAc
    constructor(parent: cdk.Construct, id: string, props: PipelineCloudFormationActionProps, configuration?: any) {


Construct initializers should never have 4 parameters. Why do we need configuration externally to props?

This is just an abstract class, so I don't think it really matters. Also, this is the way it's modeled now, I haven't touched it in this PR.

eladb · 2018-09-26T08:32:47Z

packages/@aws-cdk/aws-lambda/lib/lambda-ref.ts

+     * @returns the newly created {@link PipelineInvokeAction}
+     */
+    public addToPipeline(stage: codepipeline.IStage, name: string, props?: CommonPipelineInvokeActionProps): PipelineInvokeAction {
+        return new PipelineInvokeAction(this.parent!, name, {


Didn't we agree that we always use this here?

True. Will change.

skinny85 · 2018-09-26T16:48:03Z

@eladb any thoughts on the JSII error?

eladb · 2018-09-26T20:36:13Z

@skinny85 I think your analysis is correct, seems like this is the result of using multiple inheritance. Is there any way to simplify this hierarchy? It's not only blowing jsii's head, it's also blowing mine... Too complex.

One way to address such cases is to use containment instead of inheritance. Check out what we did in the API gateway library for example with MethodOptions and MethodProps. MethodProps simply includes an options field, and this is also what resource.addMethod exposes for customization. I believe it's a very similar use case here...

skinny85 · 2018-09-27T16:59:49Z

One way to address such cases is to use containment instead of inheritance. Check out what we did in the API gateway library for example with MethodOptions and MethodProps. MethodProps simply includes an options field, and this is also what resource.addMethod exposes for customization. I believe it's a very similar use case here...

I don't agree - that would mean that you have to do:

new SomeAction(this, 'Action', {
    stage: someStage,
    someProperty: sth,
    options: {
         runOrder: 13,
    },
});

I don't think it makes sense to change the API we want into something like that just because of a JSII quirk/bug...

Don't you think it makes sense to correct this at the JSII level, if the underlying code is valid in TypeScript? What if a customer does this while writing their own Construct library?

skinny85 · 2018-10-01T22:03:33Z

Changed the interface hierarchy to not have the same interface in the hierarchy chain twice (at the cost of more changes in the props interfaces in each package).

skinny85 · 2018-10-03T16:56:08Z

@eladb ping

RomainMuller · 2018-10-04T09:47:59Z

Is the jsii issue caused by the same interface being in the inheritance lineage twice? If so, I reckon the code generators should properly detect this and generate valid code.

skinny85 · 2018-10-04T16:43:55Z

Is the jsii issue caused by the same interface being in the inheritance lineage twice?

Yep.

If so, I reckon the code generators should properly detect this and generate valid code.

I concur.

eladb · 2018-10-04T22:23:55Z

If so, I reckon the code generators should properly detect this and generate valid code.

Can someone open a jsii issue for this?

skinny85 · 2018-10-05T01:18:02Z

Can someone open a jsii issue for this?

Done: aws/jsii#255

@eladb should I wait for the JSII fix with this PR, or should I go ahead with the workaround I did? I want to make runOrder immutable (needed for #845).

eladb · 2018-10-05T14:47:34Z

@skinny85 don't wait

skinny85 · 2018-10-05T20:49:49Z

Made runOrder immutable.

skinny85 · 2018-10-08T19:46:04Z

@eladb @RomainMuller ping

…reating Actions. Also create a new addToPipeline method on LambdaRef, which was missed when doing these for Code* services.

skinny85 · 2018-10-09T16:25:34Z

Thanks for the review @eladb!

Rebased onto the PolicyDocument changes.

Bug Fixes ========= * **aws-apigateway:** allow + in path parts ([#769](#769)) ([9aadcb6](9aadcb6)), closes [#768](#768) * **aws-cdk:** continue after exceptions in stack monitor ([#791](#791)) ([88b599d](88b599d)), closes [#787](#787) * **aws-cloudfront:** properly support loggingConfig ([#809](#809)) ([a09afc4](a09afc4)), closes [#721](#721) * **aws-ec2:** Add Burstable Generation 3 Instances ([#812](#812)) ([6c523f2](6c523f2)) * **aws-ec2:** fix typo in resource identifier ([#818](#818)) ([bebfef0](bebfef0)) * **aws-s3:** properly export bucketDomainName ([#844](#844)) ([8caa28c](8caa28c)) * **aws-sqs:** Queue.import() doesn't return a value ([#885](#885)) ([c21ebb5](c21ebb5)), closes [#879](#879) * **cdk:** fix TagManager to evaluate to undefined if no tags are included ([#882](#882)) ([96767d7](96767d7)) * Emit valid YAML-1.1 ([#876](#876)) ([3cedc0c](3cedc0c)), closes [#875](#875) * **cdk:** jsx support conflicts with React usage ([#884](#884)) ([8824356](8824356)), closes [#830](#830) * **docs:** update supported languages in README ([#819](#819), [#450](#450)) ([#820](#820)) ([7e5738f](7e5738f)) Features ======== * **aws-apigateway:** "LambdaRestApi" and "addProxy" routes ([#867](#867)) ([aa76305](aa76305)) * **aws-cdk:** add maven wrapper to java template ([#811](#811)) ([86a55a9](86a55a9)) * **aws-cloudfront:** Support Security Policy ([#804](#804)) ([8a5299a](8a5299a)), closes [#795](#795) * **aws-codedeploy:** support setting a load balancer on a Deployment Group. ([#786](#786)) ([3d1095e](3d1095e)) * **aws-codepipeline:** allow specifying the runOrder property when creating Actions. ([#776](#776)) ([bba3602](bba3602)) * **aws-dynamodb:** IAM grants support ([#870](#870)) ([1561a4d](1561a4d)) * **aws-dynamodb:** support Global Secondary Indexes ([#760](#760)) ([4980c97](4980c97)) * **aws-dynamodb:** tags support ([#814](#814)) ([644947a](644947a)) * **aws-dynamodB:** support Local Secondary Indexes ([#825](#825)) ([a67b2d9](a67b2d9)) * **aws-ec2:** support UDP port ranges in SecurityGroups ([#835](#835)) ([8215389](8215389)) * **aws-s3:** support granting public access to objects ([#886](#886)) ([d730ac6](d730ac6)), closes [#877](#877) * **cdk:** Add support for UseOnlineResharding with UpdatePolicies ([#881](#881)) ([56f0b4e](56f0b4e)) * Manage IAM permissions for (some) CFN CodePipeline actions ([#843](#843)) ([4e050c3](4e050c3)) * Resolve paths to nyc & nodeunit ([#887](#887)) ([66ff0a8](66ff0a8)) * upgrade to jsii v0.7.7 ([c231242](c231242))

BREAKING CHANGES ================ * The `cdk.App` initializer doesn't accept any arguments and the `app.run()` method does not return a `string` anymore. All AWS CDK apps in all languages would need to be modified to adhere to the new API of the `cdk.App` construct. Instead of: const app = new App(process.argv); // ERROR // add stacks process.stdout.write(app.run()); // ERROR The new usage is: const app = new App(); // add stacks app.run(); In order to interact with applications written using this version, the CDK Toolkit must also be update using: $ npm i -g aws-cdk * **aws-iam:** This change moves the `PolicyDocument`, `PolicyStatement` and all `PolicyPrincipal` classes from the @aws-cdk/cdk module and into the @aws-cdk/aws-iam module. * **jsx:** The CDK is no longer shipped with built-in support for JSX. You can still use JSX but you will have to manually configure it. Features ======== * **aws-apigateway:** "LambdaRestApi" and "addProxy" routes ([#867](#867)) ([a733bd1](a733bd1)) * **aws-cdk:** add maven wrapper to java template ([#811](#811)) ([1ee729e](1ee729e)) * **aws-cloudfront:** Support Security Policy ([#804](#804)) ([d69b1d6](d69b1d6)), closes [#795](#795) * **aws-codedeploy:** support setting a load balancer on a Deployment Group. ([#786](#786)) ([dc0af46](dc0af46)) * **aws-codepipeline:** allow specifying the runOrder property when creating Actions. ([#776](#776)) ([8302541](8302541)) * **aws-dynamodb:** IAM grants support ([#870](#870)) ([f6c7760](f6c7760)) * **aws-dynamodb:** support Global Secondary Indexes ([#760](#760)) ([737b481](737b481)) * **aws-dynamodb:** tags support ([#814](#814)) ([c76d8c1](c76d8c1)) * **aws-dynamodB:** support Local Secondary Indexes ([#825](#825)) ([fdb4974](fdb4974)) * Manage IAM permissions for (some) CFN CodePipeline actions ([#843](#843)) ([5f2cb9f](5f2cb9f)) * Resolve paths to nyc & nodeunit ([#887](#887)) ([6d71a87](6d71a87)) * upgrade to jsii v0.7.7 ([43d2d9e](43d2d9e)) * **aws-ec2:** allow configuring subnets for NAT gateway ([#874](#874)) ([958dce6](958dce6)) * **aws-ec2:** support UDP port ranges in SecurityGroups ([#835](#835)) ([6920b9c](6920b9c)) * **aws-s3:** support granting public access to objects ([#886](#886)) ([50e0c41](50e0c41)), closes [#877](#877) * **cdk:** Add support for UseOnlineResharding with UpdatePolicies ([#881](#881)) ([a95f081](a95f081)) Bug Fixes ========= * **aws-apigateway:** allow + in path parts ([#769](#769)) ([6905b7e](6905b7e)), closes [#768](#768) * **aws-cdk:** continue after exceptions in stack monitor ([#791](#791)) ([b7c244f](b7c244f)), closes [#787](#787) * **aws-cloudfront:** properly support loggingConfig ([#809](#809)) ([d279a1d](d279a1d)), closes [#721](#721) * **aws-ec2:** Add Burstable Generation 3 Instances ([#812](#812)) ([cf62e9d](cf62e9d)) * **aws-s3:** properly export bucketDomainName ([#844](#844)) ([9a53069](9a53069)) * Emit valid YAML-1.1 ([#876](#876)) ([6c98b73](6c98b73)), closes [#875](#875) * **aws-sqs:** Queue.import() doesn't return a value ([#885](#885)) ([c38c3e7](c38c3e7)), closes [#879](#879) * **cdk:** fix TagManager to evaluate to undefined if no tags are included ([#882](#882)) ([be65a04](be65a04)) * **cdk:** jsx support conflicts with React usage ([#884](#884)) ([2a979cc](2a979cc)), closes [#830](#830) * **docs:** update supported languages in README ([#819](#819), [#450](#450)) ([#820](#820)) ([1ec443e](1ec443e)) Code Refactoring ================ * **aws-iam:** move IAM classes cdk to aws-iam ([#866](#866)) ([6c58556](6c58556)), closes [#196](#196) * remove app boilerplate and improvements to cx protocol ([#868](#868)) ([7bb5a60](7bb5a60)), closes [#216](#216)

Bug Fixes --------- * **aws-apigateway:** allow + in path parts ([#769](#769)) ([0c50d27](0c50d27)), closes [#768](#768) * **aws-cdk:** continue after exceptions in stack monitor ([#791](#791)) ([b0f3298](b0f3298)), closes [#787](#787) * **aws-cloudfront:** check for undefined and determining of the defaultRootObject prop is set or not ([#801](#801)) ([32a74c6](32a74c6)) * **aws-cloudfront:** properly support loggingConfig ([#809](#809)) ([5512f70](5512f70)), closes [#721](#721) * **aws-codecommit:** typo in README ([#780](#780)) ([0e79c2d](0e79c2d)) * **aws-ec2:** Add Burstable Generation 3 Instances ([#812](#812)) ([d36ee6d](d36ee6d)) * **aws-ec2:** fix capitalization of "VPCEndpointType" to "VpcEndpointType" ([#789](#789)) ([7a8ee2c](7a8ee2c)), closes [#765](#765) * **aws-ec2:** fix typo in resource identifier ([#818](#818)) ([f529c80](f529c80)) * **aws-elbv2:** fix load balancer registration ([#890](#890)) ([8cc9abe](8cc9abe)) * **aws-s3:** properly export bucketDomainName ([#844](#844)) ([a65060d](a65060d)) * **aws-sqs:** Queue.import() doesn't return a value ([#885](#885)) ([c592b7f](c592b7f)), closes [#879](#879) * **cdk:** fix TagManager to evaluate to undefined if no tags are included ([#882](#882)) ([477c827](477c827)) * **cdk:** init templates were not upgraded to typescript ^3.0.0 ([#904](#904)) ([2cc7475](2cc7475)) * **cdk:** jsx support conflicts with React usage ([#884](#884)) ([76d8031](76d8031)), closes [#830](#830) * **cfn2ts:** expect Token instead of CloudFormationToken ([#896](#896)) ([6eee1d2](6eee1d2)) * **docs:** fix issue [#718](#718) (Aurora DB example) ([#783](#783)) ([016f3a8](016f3a8)) * **docs:** update supported languages in README ([#819](#819), [#450](#450)) ([#820](#820)) ([ffac98c](ffac98c)) * Correct heading level of CHANGELOG.md 0.10.0 ([40d9ef0](40d9ef0)) * Emit valid YAML-1.1 ([#876](#876)) ([ff857ea](ff857ea)), closes [#875](#875) * **toolkit:** improve error message for large templates ([#900](#900)) ([a41f48f](a41f48f)), closes [#34](#34) Code Refactoring ---------------- * **aws-iam:** move IAM classes cdk to aws-iam ([#866](#866)) ([d46a95b](d46a95b)), closes [#196](#196) * **util:** remove [@aws-cdk](https://github.com/aws-cdk)/util ([#745](#745)) ([10015cb](10015cb)), closes [#709](#709) * **framework:** remove app boilerplate and improvements to cx protocol ([#868](#868)) ([005beec](005beec)), closes [#216](#216) Features -------- * **aws-apigateway:** "LambdaRestApi" and "addProxy" routes ([#867](#867)) ([905a95d](905a95d)) * **aws-cdk:** add maven wrapper to java template ([#811](#811)) ([72aa872](72aa872)) * **aws-cloudformation:** rename the CFN CodePipeline Actions. ([#771](#771)) ([007e7b4](007e7b4)) * **aws-cloudformation:** update the ReadMe of the module to reflect the new Action names. ([#775](#775)) ([6c0e75b](6c0e75b)), closes [#771](#771) * **aws-cloudfront:** Support Security Policy ([#804](#804)) ([b39bf11](b39bf11)), closes [#795](#795) * **aws-codedeploy:** Add the auto-scaling groups property to ServerDeploymentGroup. ([#739](#739)) ([0b28886](0b28886)) * **aws-codedeploy:** Deployment Configuration Construct. ([#653](#653)) ([e6b67ad](e6b67ad)) * **aws-codedeploy:** support setting a load balancer on a Deployment Group. ([#786](#786)) ([e7af9f5](e7af9f5)) * **aws-codepipeline:** allow specifying the runOrder property when creating Actions. ([#776](#776)) ([d146c8d](d146c8d)) * **aws-codepipeline, aws-codecommit, aws-s3:** change the convention for naming the source Actions to XxxSourceAction. ([#753](#753)) ([9c3ce7f](9c3ce7f)) * **aws-dynamodb:** IAM grants support ([#870](#870)) ([c5a4200](c5a4200)) * **aws-dynamodb:** support Global Secondary Indexes ([#760](#760)) ([3601440](3601440)) * **aws-dynamodb:** tags support ([#814](#814)) ([924c84e](924c84e)) * **aws-dynamodB:** support Local Secondary Indexes ([#825](#825)) ([3175af3](3175af3)) * **aws-ec2:** add support for ICMP protocol's classification Types & Codes to SecurityGroupRule ([#893](#893)) ([85bd3c0](85bd3c0)) * **aws-ec2:** allow configuring subnets for NAT gateway ([#874](#874)) ([8ec761c](8ec761c)) * **aws-ec2:** support UDP port ranges in SecurityGroups ([#835](#835)) ([b42ef90](b42ef90)) * **aws-elasticloadbalancingv2:** support for ALB/NLB ([#750](#750)) ([bd9ee01](bd9ee01)) * **aws-s3:** support granting public access to objects ([#886](#886)) ([bdee191](bdee191)), closes [#877](#877) * **cdk:** Add support for UseOnlineResharding with UpdatePolicies ([#881](#881)) ([1f717e1](1f717e1)) * **cdk:** configurable default SSM context provider ([#889](#889)) ([353412b](353412b)) * **core:** resource overrides (escape hatch) ([#784](#784)) ([5054eef](5054eef)), closes [#606](#606) * **aws-codepipeline**: Manage IAM permissions for (some) CFN CodePipeline actions ([#843](#843)) ([4c69118](4c69118)) * **toolkit:** Stop creating 'empty' stacks ([#779](#779)) ([1dddd8a](1dddd8a)) * **aws-autoscaling, aws-ec2:** Tagging support for AutoScaling/SecurityGroup ([#766](#766)) ([3d48eb2](3d48eb2)) ### BREAKING CHANGES * **framework:** The `cdk.App` constructor doesn't accept any arguments, and `app.run()` does not return a `string` anymore. All AWS CDK apps in all languages would need to be modified to adhere to the new API of the `cdk.App` construct. Instead of: const app = new App(process.argv); // ERROR // add stacks process.stdout.write(app.run()); // ERROR The new usage is: const app = new App(); // add stacks app.run(); * **framework:** The CDK is no longer shipped with built-in support for JSX. You can still use JSX but you will have to manually configure it. * **aws-iam:** `PolicyDocument`, `PolicyStatement` and all `PolicyPrincipal` classes moved from the @aws-cdk/cdk module and into the @aws-cdk/aws-iam module. * **aws-codepipeline-api**: `Artifact.subartifact` method of the CodePipeline API was renamed to `Artifact.atPath`. * constructor signature of `TagManager` has changed. `initialTags` is now passed inside a props object. * **util:** @aws-cdk/util is no longer available * **aws-elasticloadbalancingv2:** Adds classes for modeling Application and Network Load Balancers. AutoScalingGroups now implement the interface that makes constructs a load balancing target. The breaking change is that Security Group rule identifiers have been changed in order to make adding rules more reliable. No code changes are necessary but existing deployments may experience unexpected changes. * **aws-cloudformation:** this renames all CloudFormation Actions for CodePipeline to bring them in line with Actions defined in other service packages. * **aws-codepipeline, aws-codecommit, aws-s3:** change the names of the source Actions from XxxSource to XxxSourceAction. This is to align them with the other Actions, like Build. Also, CodeBuild has the concept of Sources, so it makes sense to strongly differentiate between the two.

skinny85 requested a review from eladb September 25, 2018 23:06

eladb reviewed Sep 26, 2018

View reviewed changes

skinny85 force-pushed the feature/run-order branch from 7b34e97 to a16c6be Compare October 1, 2018 22:02

skinny85 force-pushed the feature/run-order branch from a16c6be to 53a9eef Compare October 1, 2018 22:12

skinny85 mentioned this pull request Oct 5, 2018

Correctly handle multiple interface inheritance aws/jsii#255

Closed

skinny85 force-pushed the feature/run-order branch from 53a9eef to f34b50f Compare October 5, 2018 20:49

eladb approved these changes Oct 9, 2018

View reviewed changes

feat(aws-codepipeline): allow specifying the runOrder property when c…

250689c

…reating Actions. Also create a new addToPipeline method on LambdaRef, which was missed when doing these for Code* services.

skinny85 force-pushed the feature/run-order branch from f34b50f to 250689c Compare October 9, 2018 16:24

skinny85 merged commit bba3602 into aws:master Oct 9, 2018

skinny85 deleted the feature/run-order branch October 9, 2018 16:50

eladb mentioned this pull request Oct 10, 2018

v0.11.0 #888

Merged

NGL321 added the contribution/core This is a PR that came from AWS. label Sep 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow specifying the runOrder property when creating CodePipeline Actions #776

Allow specifying the runOrder property when creating CodePipeline Actions #776

skinny85 commented Sep 25, 2018

skinny85 commented Sep 26, 2018

eladb Sep 26, 2018

skinny85 Sep 26, 2018

eladb Sep 26, 2018

skinny85 Sep 26, 2018

skinny85 commented Sep 26, 2018

eladb commented Sep 26, 2018

skinny85 commented Sep 27, 2018

skinny85 commented Oct 1, 2018

skinny85 commented Oct 3, 2018

RomainMuller commented Oct 4, 2018

skinny85 commented Oct 4, 2018

eladb commented Oct 4, 2018

skinny85 commented Oct 5, 2018

eladb commented Oct 5, 2018

skinny85 commented Oct 5, 2018

skinny85 commented Oct 8, 2018

skinny85 commented Oct 9, 2018

		@@ -48,6 +48,7 @@ export abstract class PipelineCloudFormationAction extends codepipeline.DeployAc
		constructor(parent: cdk.Construct, id: string, props: PipelineCloudFormationActionProps, configuration?: any) {

Allow specifying the runOrder property when creating CodePipeline Actions #776

Allow specifying the runOrder property when creating CodePipeline Actions #776

Conversation

skinny85 commented Sep 25, 2018

skinny85 commented Sep 26, 2018

eladb Sep 26, 2018

Choose a reason for hiding this comment

skinny85 Sep 26, 2018

Choose a reason for hiding this comment

eladb Sep 26, 2018

Choose a reason for hiding this comment

skinny85 Sep 26, 2018

Choose a reason for hiding this comment

skinny85 commented Sep 26, 2018

eladb commented Sep 26, 2018

skinny85 commented Sep 27, 2018

skinny85 commented Oct 1, 2018

skinny85 commented Oct 3, 2018

RomainMuller commented Oct 4, 2018

skinny85 commented Oct 4, 2018

eladb commented Oct 4, 2018

skinny85 commented Oct 5, 2018

eladb commented Oct 5, 2018

skinny85 commented Oct 5, 2018

skinny85 commented Oct 8, 2018

skinny85 commented Oct 9, 2018