fix(monorepos): github-release not created (#669)

fixes a couple of issues:
1. if node releaser doesn't specify packageName then github-release
wasn't filtering merged PRs correctly.
2. when solving the above, we need to be consistent about the
relationship between packageName and the release branch name.


Fixes #668

__snapshots__/github-release.js

@@ -146,3 +146,15 @@ exports['GitHubRelease createRelease creates releases for submodule in monorepo
     'autorelease: tagged'
   ]
 }
+
+exports['GitHubRelease createRelease attempts to guess package name for submodule release 1'] = {
+  'tag_name': '@google-cloud/foo-v1.0.3',
+  'body': '\n* entry',
+  'name': '@google-cloud/foo @google-cloud/foo-v1.0.3'
+}
+
+exports['GitHubRelease createRelease attempts to guess package name for submodule release 2'] = {
+  'labels': [
+    'autorelease: tagged'
+  ]
+}

src/github-release.ts

@@ -14,6 +14,7 @@
 
 import chalk = require('chalk');
 import {checkpoint, CheckpointType} from './util/checkpoint';
+import {packageBranchPrefix} from './util/package-branch-prefix';
 import {ReleasePRFactory} from './release-pr-factory';
 import {GitHub, OctokitAPIs} from './github';
 import {parse} from 'semver';
@@ -78,14 +79,29 @@ export class GitHubRelease {
   }
 
   async createRelease(): Promise<ReleaseResponse | undefined> {
+    // Attempt to lookup the package name from a well known location, such
+    // as package.json, if none is provided:
+    if (!this.packageName && this.releaseType) {
+      this.packageName = await ReleasePRFactory.class(
+        this.releaseType
+      ).lookupPackageName(this.gh, this.path);
+    }
+    if (this.packageName === undefined) {
+      throw Error(
+        `could not determine package name for release repo = ${this.repoUrl}`
+      );
+    }
+
     // In most configurations, createRelease() should be called close to when
     // a release PR is merged, e.g., a GitHub action that kicks off this
     // workflow on merge. For tis reason, we can pull a fairly small number of PRs:
     const pageSize = 25;
     const gitHubReleasePR = await this.gh.findMergedReleasePR(
       this.labels,
       pageSize,
-      this.monorepoTags ? this.packageName : undefined
+      this.monorepoTags
+        ? packageBranchPrefix(this.packageName, this.releaseType)
+        : undefined
     );
     if (!gitHubReleasePR) {
       checkpoint('no recent release PRs found', CheckpointType.Failure);
@@ -111,24 +127,11 @@ export class GitHubRelease {
       `found release notes: \n---\n${chalk.grey(latestReleaseNotes)}\n---\n`,
       CheckpointType.Success
     );
-
-    // Attempt to lookup the package name from a well known location, such
-    // as package.json, if none is provided:
-    if (!this.packageName && this.releaseType) {
-      this.packageName = await ReleasePRFactory.class(
-        this.releaseType
-      ).lookupPackageName(this.gh);
-    }
     // Go uses '/' for a tag separator, rather than '-':
     let tagSeparator = '-';
     if (this.releaseType) {
       tagSeparator = ReleasePRFactory.class(this.releaseType).tagSeparator();
     }
-    if (this.packageName === undefined) {
-      throw Error(
-        `could not determine package name for release repo = ${this.repoUrl}`
-      );
-    }
 
     const release = await this.gh.createRelease(
       this.packageName,

src/github.ts

@@ -551,10 +551,12 @@ export class GitHub {
   async findMergedReleasePR(
     labels: string[],
     perPage = 100,
-    prefix: string | undefined = undefined,
+    branchPrefix: string | undefined = undefined,
     preRelease = true
   ): Promise<GitHubReleasePR | undefined> {
-    prefix = prefix?.endsWith('-') ? prefix.replace(/-$/, '') : prefix;
+    branchPrefix = branchPrefix?.endsWith('-')
+      ? branchPrefix.replace(/-$/, '')
+      : branchPrefix;
     const baseLabel = await this.getBaseLabel();
     const pullsResponse = (await this.request(
       `GET /repos/:owner/:repo/pulls?state=closed&per_page=${perPage}${
@@ -594,9 +596,9 @@ export class GitHub {
         // it's easiest/safest to just pull this out by string search.
         const version = match[2];
         if (!version) continue;
-        if (prefix && match[1] !== prefix) {
+        if (branchPrefix && match[1] !== branchPrefix) {
           continue;
-        } else if (!prefix && match[1]) {
+        } else if (!branchPrefix && match[1]) {
           continue;
         }
 

src/release-pr.ts

@@ -23,6 +23,7 @@ type PullsListResponseItems = PromiseValue<
 import * as semver from 'semver';
 
 import {checkpoint, CheckpointType} from './util/checkpoint';
+import {packageBranchPrefix} from './util/package-branch-prefix';
 import {ConventionalCommits} from './conventional-commits';
 import {GitHub, GitHubTag, OctokitAPIs} from './github';
 import {Commit} from './graphql-to-commits';
@@ -98,6 +99,7 @@ export class ReleasePR {
   snapshot?: boolean;
   lastPackageVersion?: string;
   changelogSections?: [];
+  releaseType: string;
 
   constructor(options: ReleasePROptions) {
     this.bumpMinorPreMajor = options.bumpMinorPreMajor || false;
@@ -123,6 +125,7 @@ export class ReleasePR {
     this.gh = this.gitHubInstance(options.octokitAPIs);
 
     this.changelogSections = options.changelogSections;
+    this.releaseType = options.releaseType;
   }
 
   async run(): Promise<number | undefined> {
@@ -186,8 +189,12 @@ export class ReleasePR {
   // A releaser can implement this method to automatically detect
   // the release name when creating a GitHub release, for instance by returning
   // name in package.json, or setup.py.
-  // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  static async lookupPackageName(gh: GitHub): Promise<string | undefined> {
+  static async lookupPackageName(
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    gh: GitHub,
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    path?: string
+  ): Promise<string | undefined> {
     return Promise.resolve(undefined);
   }
 
@@ -273,11 +280,10 @@ export class ReleasePR {
     const updates = options.updates;
     const version = options.version;
     const includePackageName = options.includePackageName;
-    // Do not include npm style @org/ prefixes in the branch name:
-    const branchPrefix = this.packageName.match(/^@[\w-]+\//)
-      ? this.packageName.split('/')[1]
-      : this.packageName;
-
+    const branchPrefix = packageBranchPrefix(
+      this.packageName,
+      this.releaseType
+    );
     const title = includePackageName
       ? `chore: release ${this.packageName} ${version}`
       : `chore: release ${version}`;
@@ -321,13 +327,17 @@ export class ReleasePR {
     return changelogEntry.split('\n').length === 1;
   }
 
-  addPath(file: string) {
-    if (this.path === undefined) {
+  static addPathStatic(file: string, path?: string) {
+    if (path === undefined) {
       return file;
     } else {
-      const path = this.path.replace(/[/\\]$/, '');
+      path = path.replace(/[/\\]$/, '');
       file = file.replace(/^[/\\]/, '');
       return `${path}/${file}`;
     }
   }
+
+  addPath(file: string) {
+    return ReleasePR.addPathStatic(file, this.path);
+  }
 }

src/releasers/node.ts

@@ -17,6 +17,7 @@ import {ReleasePR, ReleaseCandidate} from '../release-pr';
 import {ConventionalCommits} from '../conventional-commits';
 import {GitHub, GitHubTag, GitHubFileContents} from '../github';
 import {checkpoint, CheckpointType} from '../util/checkpoint';
+import {packageBranchPrefix} from '../util/package-branch-prefix';
 import {Update} from '../updaters/update';
 import {Commit} from '../graphql-to-commits';
 
@@ -29,8 +30,18 @@ import {SamplesPackageJson} from '../updaters/samples-package-json';
 export class Node extends ReleasePR {
   static releaserName = 'node';
   protected async _run(): Promise<number | undefined> {
+    // Make an effort to populate packageName from the contents of
+    // the package.json, rather than forcing this to be set:
+    const contents: GitHubFileContents = await this.gh.getFileContents(
+      this.addPath('package.json')
+    );
+    const pkg = JSON.parse(contents.parsedContent);
+    if (pkg.name) this.packageName = pkg.name;
+
     const latestTag: GitHubTag | undefined = await this.gh.latestTag(
-      this.monorepoTags ? `${this.packageName}-` : undefined
+      this.monorepoTags
+        ? `${packageBranchPrefix(this.packageName, 'node')}-`
+        : undefined
     );
     const commits: Commit[] = await this.commits({
       sha: latestTag ? latestTag.sha : undefined,
@@ -68,14 +79,6 @@ export class Node extends ReleasePR {
 
     const updates: Update[] = [];
 
-    // Make an effort to populate packageName from the contents of
-    // the package.json, rather than forcing this to be set:
-    const contents: GitHubFileContents = await this.gh.getFileContents(
-      this.addPath('package.json')
-    );
-    const pkg = JSON.parse(contents.parsedContent);
-    if (pkg.name) this.packageName = pkg.name;
-
     updates.push(
       new PackageJson({
         path: this.addPath('package-lock.json'),
@@ -125,11 +128,14 @@ export class Node extends ReleasePR {
   // A releaser can implement this method to automatically detect
   // the release name when creating a GitHub release, for instance by returning
   // name in package.json, or setup.py.
-  static async lookupPackageName(gh: GitHub): Promise<string | undefined> {
+  static async lookupPackageName(
+    gh: GitHub,
+    path?: string
+  ): Promise<string | undefined> {
     // Make an effort to populate packageName from the contents of
     // the package.json, rather than forcing this to be set:
     const contents: GitHubFileContents = await gh.getFileContents(
-      'package.json'
+      this.addPathStatic('package.json', path)
     );
     const pkg = JSON.parse(contents.parsedContent);
     if (pkg.name) return pkg.name;

src/util/package-branch-prefix.ts

@@ -0,0 +1,30 @@
+// Copyright 2021 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// map from a packageName to the prefix used in release branch creation.
+export function packageBranchPrefix(packageName: string, releaseType?: string) {
+  let branchPrefix: string;
+  switch (releaseType) {
+    case 'node': {
+      branchPrefix = packageName.match(/^@[\w-]+\//)
+        ? packageName.split('/')[1]
+        : packageName;
+      break;
+    }
+    default: {
+      branchPrefix = packageName;
+    }
+  }
+  return branchPrefix;
+}