feat(codecommit): make Repository a source for CodeStar Notifications #15739

badfun · 2021-07-23T21:19:57Z

Fixes #15653

Notification rule can be created for CodeCommit Repository events using @aws-cdk/aws-codestarnotifications

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

…b.com/badfun/aws-cdk into add-codecommit-to-notification-source

gitpod-io · 2021-07-23T21:20:00Z

mergify · 2021-07-23T21:20:31Z

Title does not follow the guidelines of Conventional Commits. Please adjust title before merge.

badfun · 2021-07-23T21:36:40Z

@luckily @skinny85 - I have re-submitted the pull request for issue #15653 here.

skinny85

Looks great @badfun, thanks for the contribution!

A few comments, mainly around naming.

skinny85 · 2021-07-27T22:38:11Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

@@ -110,8 +123,128 @@ export interface IRepository extends IResource {
   * Grant the given identity permissions to read this repository.
   */
  grantRead(grantee: iam.IGrantable): iam.Grant;
+
+


One empty line too many 🙂.

skinny85 · 2021-07-27T22:38:21Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

+
+
+  /**
+   *


No need for this empty line.

skinny85 · 2021-07-27T22:39:48Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

+  /**
+   * Defines a CodeStar Notification rule which triggers when a comment is made on a commit.
+   */
+  notifyOnCommentOnCommits(


Suggested change

notifyOnCommentOnCommits(

notifyOnCommitComment(

skinny85 · 2021-07-27T22:40:07Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

+  /**
+   * Defines a CodeStar Notification rule which triggers when a comment is made on a pull request
+   */
+  notifyOnCommentOnPullRequests(


Suggested change

notifyOnCommentOnPullRequests(

notifyOnPullRequestsComment(

Apologies, this should actually be

Suggested change

notifyOnCommentOnPullRequests(

notifyOnPullRequestComment(

skinny85 · 2021-07-27T22:41:55Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

+  /**
+   * Defines a CodeStar Notification rule which triggers when an approval status is changed
+   */
+  notifyOnApprovalsStatusChanged(


Suggested change

notifyOnApprovalsStatusChanged(

notifyOnApprovalStatusChanged(

Shouldn't this stay plural, to match the event itself: 'codecommit-repository-approvals-status-changed'?

I prefer the singular myself for camel-case, but the event is plural.

I'm fine using a different name.

skinny85 · 2021-07-27T22:47:22Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

+  /**
+   * Trigger notification when a branch or tag is created
+   */
+  BRANCHES_AND_TAGS_CREATED = 'codecommit-repository-branches-and-tags-created',


Suggested change

BRANCHES_AND_TAGS_CREATED = 'codecommit-repository-branches-and-tags-created',

BRANCH_OR_TAG_CREATED = 'codecommit-repository-branches-and-tags-created',

skinny85 · 2021-07-27T22:47:36Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

+  /**
+   * Trigger notification when a branch or tag is deleted
+   */
+  BRANCHES_AND_TAGS_DELETED = 'codecommit-repository-branches-and-tags-deleted',


Suggested change

BRANCHES_AND_TAGS_DELETED = 'codecommit-repository-branches-and-tags-deleted',

BRANCH_OR_TAG_DELETED = 'codecommit-repository-branches-and-tags-deleted',

skinny85 · 2021-07-27T22:48:22Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

 import * as events from '@aws-cdk/aws-events';
 import * as iam from '@aws-cdk/aws-iam';
 import { IResource, Lazy, Resource, Stack } from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import { CfnRepository } from './codecommit.generated';

-export interface IRepository extends IResource {
+/**
+ * Additional options to pass to the notification rule


Docs should be complete sentences, with fullstops.

Same comment everywhere in the PR.

Still not resolved I see 🙂.

skinny85 · 2021-07-27T22:49:06Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

+  /**
+   * Trigger notification when a branch or tag is updated
+   */
+  BRANCHES_AND_TAGS_UPDATED = 'codecommit-repository-branches-and-tags-updated',


Suggested change

BRANCHES_AND_TAGS_UPDATED = 'codecommit-repository-branches-and-tags-updated',

BRANCH_OR_TAG_UPDATED = 'codecommit-repository-branches-and-tags-updated',

skinny85 · 2021-07-27T22:49:35Z

packages/@aws-cdk/aws-codecommit/package.json

@@ -98,7 +99,8 @@
    "@aws-cdk/aws-events": "0.0.0",
    "@aws-cdk/aws-iam": "0.0.0",
    "@aws-cdk/core": "0.0.0",
-    "constructs": "^3.3.69"
+    "constructs": "^3.3.69",
+    "@aws-cdk/aws-codestarnotifications": "0.0.0"


Please put this at the beginning of the list, like you did for "dependencies".

badfun · 2021-07-29T20:05:26Z

Thanks @skinny85,
I will make the changes asap.

skinny85 · 2021-07-30T00:17:33Z

@badfun please re-request my review (there's a button in the top-right of the PR window, next to my avatar) once you're ready for another round of reviews. Thanks for the contribution!

Pull request has been modified.

badfun · 2021-07-30T16:07:43Z

@skinny85 I've made most of the changes. Looking for some clarification on a few points commented above.
thanks,
Ken

skinny85

Let's change the names according to my comments (I'm fine with the fact that they are not identical to the event names).

skinny85 · 2021-07-30T18:22:29Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

 import * as events from '@aws-cdk/aws-events';
 import * as iam from '@aws-cdk/aws-iam';
 import { IResource, Lazy, Resource, Stack } from '@aws-cdk/core';
 import { Construct } from 'constructs';
 import { CfnRepository } from './codecommit.generated';

-export interface IRepository extends IResource {
+/**
+ * Additional options to pass to the notification rule


Still not resolved I see 🙂.

packages/@aws-cdk/aws-codecommit/lib/repository.ts

skinny85 · 2021-07-30T18:23:47Z

packages/@aws-cdk/aws-codecommit/lib/repository.ts

 }

+


Still here.

Pull request has been modified.

…b.com/badfun/aws-cdk into add-codecommit-to-notification-source

badfun · 2021-08-01T23:13:57Z

@skinny85 I believe I got everything now. Let me know if there is something I've missed.

packages/@aws-cdk/aws-codecommit/lib/repository.ts

skinny85

Thanks for the contribution @badfun!

mergify · 2021-08-25T20:05:01Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2021-08-25T20:46:12Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2021-08-25T20:46:16Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: b95a5c9
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

…aws#15739) ---- Fixes aws#15653 Notification rule can be created for CodeCommit Repository events using @aws-cdk/aws-codestarnotifications *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

kdirschl and others added 11 commits July 20, 2021 19:08

Added CodeCommit repository as source for notification rule

ce19080

In progress. Pull request create and merge only events so far

8e21ebf

Added rest of events to ENUM and RepositoryBase

62a1b69

updated IRepository interface and read me

0e1f890

test push from WSL2

373ab8a

Merge branch 'aws:master' into add-codecommit-to-notification-source

ee6f687

tests in progress

652007f

Merge branch 'add-codecommit-to-notification-source' of https://githu…

554d2be

…b.com/badfun/aws-cdk into add-codecommit-to-notification-source

Integration tests now passing

3838f2c

Merge branch 'aws:master' into add-codecommit-to-notification-source

44f7960

changed name of test to match existing

bdabea8

badfun changed the title ~~fix(aws-codecommit) : added codestarnotifications to repository construct~~ fix(aws-codecommit): added codestarnotifications to repository construct Jul 23, 2021

badfun marked this pull request as ready for review July 23, 2021 21:32

skinny85 self-assigned this Jul 25, 2021

skinny85 previously requested changes Jul 27, 2021

View reviewed changes

Merge branch 'aws:master' into add-codecommit-to-notification-source

1ff675a

suggested changes in progress

8137be8

skinny85 previously requested changes Jul 30, 2021

View reviewed changes

skinny85 changed the title ~~fix(aws-codecommit): added codestarnotifications to repository construct~~ feat(codecommit): make Repository a source for CodeStar Notifications Jul 30, 2021

badfun added 2 commits August 1, 2021 19:08

completed suggested changes

bcba6cd

Merge branch 'aws:master' into add-codecommit-to-notification-source

15b7231

Merge branch 'add-codecommit-to-notification-source' of https://githu…

939ecb0

…b.com/badfun/aws-cdk into add-codecommit-to-notification-source

badfun requested a review from skinny85 August 1, 2021 23:13

skinny85 reviewed Aug 25, 2021

View reviewed changes

packages/@aws-cdk/aws-codecommit/lib/repository.ts Outdated Show resolved Hide resolved

skinny85 reviewed Aug 25, 2021

View reviewed changes

packages/@aws-cdk/aws-codecommit/lib/repository.ts Outdated Show resolved Hide resolved

Remove notifyOnCommitComment()

e0a4028

skinny85 approved these changes Aug 25, 2021

View reviewed changes

Merge branch 'master' into add-codecommit-to-notification-source

b95a5c9

mergify bot merged commit ae34d4a into aws:master Aug 25, 2021

badfun deleted the add-codecommit-to-notification-source branch August 25, 2021 21:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(codecommit): make Repository a source for CodeStar Notifications #15739

feat(codecommit): make Repository a source for CodeStar Notifications #15739

badfun commented Jul 23, 2021 •

edited

Loading

gitpod-io bot commented Jul 23, 2021

mergify bot commented Jul 23, 2021

badfun commented Jul 23, 2021

skinny85 left a comment

skinny85 Jul 27, 2021

skinny85 Jul 27, 2021

skinny85 Jul 27, 2021

skinny85 Jul 27, 2021

skinny85 Jul 30, 2021

skinny85 Jul 27, 2021

badfun Jul 30, 2021

skinny85 Jul 30, 2021

skinny85 Jul 27, 2021

skinny85 Jul 27, 2021

skinny85 Jul 27, 2021

skinny85 Jul 30, 2021

skinny85 Jul 27, 2021

skinny85 Jul 27, 2021

badfun commented Jul 29, 2021

skinny85 commented Jul 30, 2021

badfun commented Jul 30, 2021

skinny85 left a comment

skinny85 Jul 30, 2021

skinny85 Jul 30, 2021

badfun commented Aug 1, 2021

skinny85 left a comment

mergify bot commented Aug 25, 2021

mergify bot commented Aug 25, 2021

aws-cdk-automation commented Aug 25, 2021

	notifyOnApprovalsStatusChanged(
	notifyOnApprovalStatusChanged(

	BRANCHES_AND_TAGS_CREATED = 'codecommit-repository-branches-and-tags-created',
	BRANCH_OR_TAG_CREATED = 'codecommit-repository-branches-and-tags-created',

	BRANCHES_AND_TAGS_DELETED = 'codecommit-repository-branches-and-tags-deleted',
	BRANCH_OR_TAG_DELETED = 'codecommit-repository-branches-and-tags-deleted',

	BRANCHES_AND_TAGS_UPDATED = 'codecommit-repository-branches-and-tags-updated',
	BRANCH_OR_TAG_UPDATED = 'codecommit-repository-branches-and-tags-updated',

feat(codecommit): make Repository a source for CodeStar Notifications #15739

feat(codecommit): make Repository a source for CodeStar Notifications #15739

Conversation

badfun commented Jul 23, 2021 • edited Loading

gitpod-io bot commented Jul 23, 2021

mergify bot commented Jul 23, 2021

badfun commented Jul 23, 2021

skinny85 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

badfun commented Jul 29, 2021

skinny85 commented Jul 30, 2021

badfun commented Jul 30, 2021

skinny85 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

badfun commented Aug 1, 2021

skinny85 left a comment

Choose a reason for hiding this comment

mergify bot commented Aug 25, 2021

mergify bot commented Aug 25, 2021

aws-cdk-automation commented Aug 25, 2021

AWS CodeBuild CI Report

badfun commented Jul 23, 2021 •

edited

Loading