Add automatic release-note-generation

[Mojken]

Adds a setting to generate release notes automatically, using Github's built-in generator (which is based on pull-requests). Posts the description in the comments so that reviewers can see it, and creates a draft instead of a complete release to allow for amending.

You can find a proof-of-concept run at this test repo.
Closes #26

[kjagiello]

Thanks a lot for your contribution! Great work! 🎉 Leaving some thoughts below for you to consider.

[kjagiello]

Let's not mutate the PR body and return it instead.

[Mojken]

Because I now mutate the actual body, I'm keeping this line in to prevent stale data in the pullRequest variable. If I don't, validateBody will tell you the description is missing when it isn't. I hope this is fine.

[hannseman]

I too really think we should avoid mutating the PR object.

If I don't, validateBody will tell you the description is missing when it isn't.

IMO, validateBody shouldn't throw an error if generate_release_notes is set to true.

[Mojken]

That's how it used to work, but after this last revision, the rest of the code is completely unaware that the description wasn't written by people. If you think not mutating the object and instead having validateBody check, that's an easy fix, so I can go ahead and do that, but I don't know if I agree that it's the best solution. Under normal circumstances I agree that mutating the object is bad practice, but considering we just mutated the actual PR, I don't see it being an issue. You make the call, I don't know what is better for this code at the end of the day.

[hannseman]

I think we can be more explicit by adding a releaseNotes argument to the validate function, something like this (I have not tested this):

diff --git a/src/action.js b/src/action.js
index 73ae4e7..d454235 100644
--- a/src/action.js
+++ b/src/action.js
@@ -19,15 +19,19 @@ async function action() {
     core.info(`Validating pull request for action ${action}.`);
 
     // If the release notes should have changed, generate new ones
+    let releaseNotes;
     if (
       JSON.parse(core.getInput("generate_release_notes") || false) === true && //Should we generate notes?
       (action === "opened" || //Was the action "opened" or
         (action === "edited" && //Was it "edited" and
           github.context.payload.changes.body.from == pullRequest.body)) //The edit wasn't of the description
-    )
-      await generateReleaseNotes(pullRequest);
+    ) {
+      releaseNotes = await generateReleaseNotes(pullRequest);
+    } else {
+      releaseNotes = pullRequest.body;
+    }
 
-    await validate(pullRequest);
+    await validate(pullRequest, releaseNotes);
   } else if (action === "closed" && pullRequest.merged) {
     // Previously validated PR was merged so create release
     core.info(`Creating release.`);
@@ -42,7 +46,7 @@ async function action() {
   core.setOutput("release", pullRequest.title);
 }
 
-async function validate(pullRequest) {
+async function validate(pullRequest, releaseNotes) {
   const doValidate = JSON.parse(core.getInput("validate") || true) === true;
 
   if (!doValidate) {
@@ -55,7 +59,7 @@ async function validate(pullRequest) {
     // Check title
     validateTitle(pullRequest);
     // Check body
-    validateBody(pullRequest);
+    validateReleaseNotes(releaseNotes);
     // Check that PR is from base to head
     validateBranches(pullRequest);
     // Check that release doesn't already exist
@@ -123,10 +127,9 @@ function validateTitle(pullRequest) {
   }
 }
 
-function validateBody(pullRequest) {
-  core.info("Validating body..");
-  const { body } = pullRequest;
-  if (!body) {
+function validateReleaseNotes(releaseNotes) {
+  core.info("Validating releaseNotes..");
+  if (!releaseNotes) {
     throw new ValidationError("Missing description.");
   }
 }
@@ -301,9 +304,7 @@ async function generateReleaseNotes(pullRequest) {
       issue_number: pullRequest.number,
       body: response.data.body,
     });
-
-    //pullRequest now holds stale data, updating it
-    pullRequest.body = response.data.body;
+    return response.data.body;
   } catch (error) {
     throw Error("Failed to generate a body: " + error);
   }
@@ -340,7 +341,7 @@ module.exports = {
   action,
   validate,
   validateTitle,
-  validateBody,
+  validateReleaseNotes,
   validateBranches,
   validateRelease,
   getTagName,

[hannseman]

Thanks for the PR!

[hannseman]

I too really think we should avoid mutating the PR object.

If I don't, validateBody will tell you the description is missing when it isn't.

IMO, validateBody shouldn't throw an error if generate_release_notes is set to true.

[Mojken]

Since my choice to tie the draft option to the generate release notes one confused all of the reviewers so far (it's since been changed), I figured I might as well explain my motivation for it. Looking at GitHub's guide, step 8 is:

Check the generated notes to ensure they include all (and only) the information you want to include.

I take this to mean that GitHub in no way guarantees that the notes generated are appropriate for a release as-is, and may need revision before being submitted. Since the way the workflow worked before didn't leave any opportunity for people to do that, I decided it's best to generate a draft and ask that people revise the notes there before release. We now instead edit the description in-place, which does let people edit it before a release is generated, and the release function grabs the description rather than generating a new one. I do agree that tying them together is weird, and I was probably mostly just being lazy, but it's now broken out into a separate option, and the need for it has been mostly removed (it's still useful for verifying that your changes weren't removed without you noticing it somewhere between edit and merge, if getting the right notes the first time is crucial).