chore(prlinter): revert commit message validation from (aws#6573) (aw…

…s#6678) This reverts commit 2f75318.
NovakGu · Mar 11, 2020 · c90fe9b · c90fe9b
1 parent e1fa5ca
commit c90fe9b
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 118 deletions.
diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md
@@ -1,37 +1,10 @@
-## Description
 
-<!--
-The description should describe _motivation_. Think about your code reviewers and what information they need in order to understand what you did. If it's a big commit (hopefully not), try to provide some good entry points so it will be easier to follow.
-If not obvious (i.e. from unit tests), describe how you verified that your change works.
--->
-
-## Commit Message
-<!--Simply copy paste from the PR title and replace the necessary parts-->
-{*replace-with-pr-title*} (#{*replace-with-pr-number*})
-
-<!--Use this to give a more detailed message that describes the change-->
-{replace-with-extended-commit-message}
-
-<!--For every issue your PR resolves, add `fixes #<issue>` or `closes #<issue>`-->
-
-<!--Shout out to collaborators.-->
-
-<!--If your PR includes breaking changes, uncomment and fill in the following (notice how multiple breaking changes should be formatted):-->
-<!--
-BREAKING CHANGE: Description of what broke and how to achieve this behavior now<br>
-\* **module-name:** Another breaking change<br>
-\* **module-name:** Yet another breaking change
--->
-
-<!--IMPORTANT: This section cannot contain any additional markdown headers (#)-->
-
-## End Commit Message
 
 ----
 
 *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
 
-<!--
+<!-- 
 Please read the contribution guidelines and follow the pull-request checklist:
 https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md
  -->
diff --git a/.github/actions/prlinter/index.js b/.github/actions/prlinter/index.js
@@ -3,8 +3,7 @@ const github = require('@actions/github');
 const linter = require('prlint')
 
 const checks = {
-    "MANDATORY_CHANGES": linter.mandatoryChanges,
-    "COMMIT_MESSAGE": linter.commitMessage
+    "MANDATORY_CHANGES": linter.mandatoryChanges
 }
 
 async function run() {
@@ -21,7 +20,7 @@ async function run() {
         }
 
         await check(number);
-
+    
     } catch (error) {
 
         core.setFailed(error.message);

diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
@@ -11,8 +11,9 @@ and let us know if it's not up-to-date (even better, submit a PR with your  corr
   - [Step 1: Open Issue](#step-1-open-issue)
   - [Step 2: Design (optional)](#step-2-design-optional)
   - [Step 3: Work your Magic](#step-3-work-your-magic)
-  - [Step 4: Pull Request](#step-4-pull-request)
-  - [Step 5: Merge](#step-5-merge)
+  - [Step 4: Commit](#step-4-commit)
+  - [Step 5: Pull Request](#step-5-pull-request)
+  - [Step 6: Merge](#step-6-merge)
 - [Tools](#tools)
   - [Main build scripts](#main-build-scripts)
   - [Partial build tools](#partial-build-tools)
@@ -52,7 +53,7 @@ For day-to-day development and normal contributions, the following SDKs and tool
  - [.NET Core SDK 3.0](https://www.microsoft.com/net/download)
  - [Python 3.6.5](https://www.python.org/downloads/release/python-365/)
  - [Ruby 2.5.1](https://www.ruby-lang.org/en/news/2018/03/28/ruby-2-5-1-released/)
-
+ 
 The basic commands to get the repository cloned and built locally follow:
 
 ```console
@@ -141,7 +142,7 @@ Integration tests perform a few functions in the CDK code base -
 3. (Optionally) Acts as a way to validate that constructs set up the CloudFormation resources as expected. A successful
    CloudFormation deployment does not mean that the resources are set up correctly.
 
-If you are working on a new feature that is using previously unused CloudFormation resource types, or involves
+If you are working on a new feature that is using previously unused CloudFormation resource types, or involves 
 configuring resource types across services, you need to write integration tests that use these resource types or
 features.
 
@@ -161,37 +162,48 @@ Examples:
 * [integ.destinations.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-lambda-destinations/test/integ.destinations.ts#L7)
 * [integ.token-authorizer.ts](https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk/aws-apigateway/test/authorizers/integ.token-authorizer.ts#L6)
 
-### Step 4: Pull Request
+### Step 4: Commit
 
-* Push to a GitHub fork or to a branch (naming convention: `<user>/<feature-bug-name>`)
-* Submit a Pull Request on GitHub and assign the PR for a review to the "aws/aws-cdk-team" team. The title and description will be used to format the commit message when its merged to master. This in turn, will translate to CHANGELOG entries. It is therefore important we be consistent and informative. Here is an example PR you should use as a reference: https://github.com/aws/aws-cdk/pull/6553.
+Create a commit with the proposed change changes:
 
-  ### Title
+* Commit title and message (and PR title and description) must adhere to [conventionalcommits](https://www.conventionalcommits.org).
+  * The title must begin with `feat(module): title`, `fix(module): title`, `refactor(module): title` or
+    `chore(module): title`.
+  * Title should be lowercase.
+  * No period at the end of the title.
 
-  * Must adhere to [conventionalcommits](https://www.conventionalcommits.org).
-  * The title must begin with one of:
-    - `feat(module): title`
-    - `fix(module): title`
-    - `refactor(module): title`
-    - `chore(module): title`
-  * Should be lowercase.
-  * No period at the end.
+* Commit message should describe _motivation_. Think about your code reviewers and what information they need in
+  order to understand what you did. If it's a big commit (hopefully not), try to provide some good entry points so
+  it will be easier to follow.
 
+* Commit message should indicate which issues are fixed: `fixes #<issue>` or `closes #<issue>`.
 
-  ### Description
+* Shout out to collaborators.
 
-  * Simply follow the PR template carefully.
+* If not obvious (i.e. from unit tests), describe how you verified that your change works.
 
+* If this commit includes breaking changes, they must be listed at the end in the following format (notice how multiple breaking changes should be formatted):
 
+```
+BREAKING CHANGE: Description of what broke and how to achieve this behavior now
+* **module-name:** Another breaking change
+* **module-name:** Yet another breaking change
+```
+
+### Step 5: Pull Request
+
+* Push to a GitHub fork or to a branch (naming convention: `<user>/<feature-bug-name>`)
+* Submit a Pull Requests on GitHub and assign the PR for a review to the "awslabs/aws-cdk" team.
 * Please follow the PR checklist written below. We trust our contributors to self-check, and this helps that process!
 * Discuss review comments and iterate until you get at least one “Approve”. When iterating, push new commits to the
   same branch. Usually all these are going to be squashed when you merge to master. The commit messages should be hints
   for you when you finalize your merge commit message.
-* Make sure to update the PR title/description if things change.
+* Make sure to update the PR title/description if things change. The PR title/description are going to be used as the
+  commit title/message and will appear in the CHANGELOG, so maintain them all the way throughout the process.
 
 
 
-### Step 5: Merge
+### Step 6: Merge
 
 * Make sure your PR builds successfully (we have CodeBuild setup to automatically build all PRs)
 * Once approved and tested, a maintainer will squash-merge to master and will use your PR title/description as the

diff --git a/tools/prlint/index.js b/tools/prlint/index.js
@@ -18,7 +18,7 @@ function createGitHubClient() {
     } else {
         console.log("Creating un-authenticated GitHub Client")
     }
-
+    
     return new GitHub({'token': token});
 }
 
@@ -67,10 +67,10 @@ async function mandatoryChanges(number) {
     }
 
     const gh = createGitHubClient();
-
+    
     const issues = gh.getIssues(OWNER, REPO);
     const repo = gh.getRepo(OWNER, REPO);
-
+    
     console.log(`⌛  Fetching PR number ${number}`)
     const issue = (await issues.getIssue(number)).data;
 
@@ -84,77 +84,14 @@ async function mandatoryChanges(number) {
     fixContainsTest(issue, files);
 
     console.log("✅  Success")
-
-}
-
-async function commitMessage(number) {
-
-    function validate() {
-
-        // this is the commit message mergify will use.
-        // see https://doc.mergify.io/actions.html#commit-message-and-squash-method.
-        const commitMessageSection = issue.body.match(/## Commit Message([\s|\S]*)## End Commit Message/);
-
-        if (!commitMessageSection || commitMessageSection.length !== 2) {
-            throw new LinterError("Your PR description doesn't specify the commit"
-                + " message properly. See for details.")
-        }
-
-        const commitMessage = commitMessageSection[1].trim();
-
-        const paragraphs = commitMessage.split(/\r\n\r\n|\n\n/);
-        const title = paragraphs[0];
-        const expectedCommitTitle = `${issue.title} (#${number})`
-
-        if (title !== expectedCommitTitle) {
-            throw new LinterError("First paragraph of '## Commit Message' section"
-                + ` must be: '${expectedCommitTitle}'`)
-        }
-
-        for (i in paragraphs) {
-            if (i != paragraphs.length - 1 && paragraphs[i].startsWith("BREAKING CHANGE:")) {
-                throw new LinterError("'BREAKING CHANGE:' must be specified as the last paragraph");
-            }
-        }
-    }
-
-    if (!number) {
-        throw new Error('Must provide a PR number')
-    }
-
-    const gh = createGitHubClient();
-
-    const issues = gh.getIssues(OWNER, REPO);
-
-    console.log(`⌛  Fetching PR number ${number}`)
-    const issue = (await issues.getIssue(number)).data;
-
-    const noSquash = issue.labels.some(function (l) {
-        return l.name.includes("no-squash");
-    });
-
-    if (issue.user.login === "dependabot[bot]" || issue.user.login === "dependabot-preview[bot]") {
-        // dependabot PR's are ok even without following this convention because they only contain
-        // a single commit in conventional commit form.
-        console.log("⏭️   Validation skipped because its a dependabot PR");
-    } else if (noSquash) {
-        // if the PR isn't merged as a squash commit, all this validation is irrelevant.
-        // this is the case for our automatic PR's to the 'release' branch.
-        console.log("⏭️   Validation skipped because the PR is labeled with 'no-squash'");
-    } else {
-        console.log("⌛  Validating...");
-        validate();
-    }
-
-    console.log("✅  Success")
+
 }
 
 // we don't use the 'export' prefix because github actions
 // node runtime doesn't seem to support ES6.
 // TODO need to verify this.
 module.exports.mandatoryChanges = mandatoryChanges
 module.exports.LinterError = LinterError
-module.exports.commitMessage = commitMessage
 
 require('make-runnable/custom')({
     printOutputFrame: false