feat: support adding labels to PRs #173

noahdietz · 2021-02-23T00:52:13Z

Adds support for adding labels to pull requests.

Fixes #172

codecov · 2021-02-23T00:53:28Z

Codecov Report

Merging #173 (6800502) into master (8ff45d8) will increase coverage by 0.11%.
The diff coverage is 90.27%.

@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
+ Coverage   87.98%   88.09%   +0.11%     
==========================================
  Files          23       24       +1     
  Lines        2222     2294      +72     
  Branches      170      173       +3     
==========================================
+ Hits         1955     2021      +66     
- Misses        266      272       +6     
  Partials        1        1

Impacted Files	Coverage Δ
src/bin/code-suggester.ts	`0.00% <0.00%> (ø)`
src/github-handler/index.ts	`95.00% <0.00%> (+0.26%)`	⬆️
src/bin/workflow.ts	`81.25% <100.00%> (+0.19%)`	⬆️
src/github-handler/issue-handler.ts	`100.00% <100.00%> (ø)`
src/index.ts	`99.56% <100.00%> (+0.01%)`	⬆️
src/types/index.ts	`98.28% <100.00%> (+0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ff45d8...c60d5f1. Read the comment docs.

noahdietz · 2021-02-23T17:55:27Z

What is a good way to cover the lines added to src/index.ts? I don't see tests for that method.

bcoe · 2021-02-24T18:19:58Z

What is a good way to cover the lines added to src/index.ts? I don't see tests for that method.

@noahdietz, for your issue test why don't you do this:

import {addLabels} from '../src';

This will pull in your helper from the index.ts file and should get the coverage up to 100%.

bcoe

This implementation looks really solid to me.

What's your specific use case out of curiosity?

noahdietz · 2021-02-24T18:43:49Z

@noahdietz, for your issue test why don't you do this:
import {addLabels} from '../src';
This will pull in your helper from the index.ts file and should get the coverage up to 100%.

index.ts doesn't export the addLabels function though - so this doesn't compile?

Also, I think one coverage issue is that the call to addLabels is behind a check (options.labels && options.labels.length > 0), that never gets exercised. I was thinking of moving that check into addLabels, so it just returns empty list if there are no labels to set, and the createPullRequest call site unconditionally calls it, making the line covered(?) and I can write a test for the addLabels no-op logic. WDYT?

What's your specific use case out of curiosity?

The gapic-showcase project has some code regeneration steps that run on PRs (to verify that, for example, a proto change results in compilable regenerated code, then runs tests with that newly generated code). I'm using code-suggester to open PRs with those generated changes when the original PR is merged, and I want to add the automerge label to the generated PR :) See here.

noahdietz · 2021-02-24T19:30:57Z

src/github-handler/issue-handler.ts

+  issue_number: number,
+  labels?: string[]
+): Promise<string[]> {
+  if (!labels || labels.length === 0) {


@bcoe moved the check on labels into addLabels instead of around the call site.

noahdietz · 2021-02-24T19:32:35Z

test/issues.ts

+  it('No-op undefined labels', async () => {
+    // setup
+    const stub = sandbox.stub(octokit.issues, 'addLabels').resolves();
+    // tests
+    const resultingLabels = await addLabels(
+      octokit,
+      upstream,
+      origin,
+      issue_number
+    );
+    sandbox.assert.neverCalledWith(stub, sinon.match.any);
+    expect(resultingLabels).to.deep.equal([]);
+  });
+
+  it('No-op with empty labels', async () => {


@bcoe added no-op tests.

noahdietz · 2021-02-24T19:34:00Z

test/main-make-pr.ts

@@ -116,6 +118,20 @@ describe('Make PR main function', () => {
        expect(testMaintainersCanModify).equals(maintainersCanModify);
        expect(testPrimary).equals(primary);
      },
+      addLabels: (


@bcoe Added stub helpers for what actually seems to be the main tests for createPullRequest.

noahdietz · 2021-02-24T19:36:46Z

Thanks for your review @bcoe! I tweaked things a bit WRT the no-op case fo addLabels and improved the coverage. LMK if you prefer it the way it was before.

noahdietz · 2021-02-25T16:13:17Z

Thanks guys for the reviews. I can't actually merge this PR so one of you will have to. Thanks!

🤖 I have created a release \*beep\* \*boop\* --- ## [1.9.0](https://www.github.com/googleapis/code-suggester/compare/v1.8.2...v1.9.0) (2021-02-25) ### Features * support adding labels to PRs ([#173](https://www.github.com/googleapis/code-suggester/issues/173)) ([df55616](https://www.github.com/googleapis/code-suggester/commit/df55616ebc02dc702d828d12b5746830d90e61d4)) ### Bug Fixes * **deps:** update dependency @types/yargs to v16 ([#168](https://www.github.com/googleapis/code-suggester/issues/168)) ([6cc6657](https://www.github.com/googleapis/code-suggester/commit/6cc6657fa5d5580432afd6d76df8e25eb6cff246)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

noahdietz added 6 commits February 22, 2021 16:41

feat: support adding labels to PRs

526dde6

add new line at eof

08632bb

fix lint errors

fc39769

add fix changes

d477b0b

another new line at eof

eaa70c2

fix license year

2bdf70b

noahdietz requested a review from a team February 23, 2021 00:52

noahdietz requested a review from a team as a code owner February 23, 2021 00:52

google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Feb 23, 2021

noahdietz changed the title ~~Add labels~~ feat: support adding labels to PRs Feb 23, 2021

bcoe approved these changes Feb 24, 2021

View reviewed changes

fix: no-op addLabels with empty labels

89c7c08

noahdietz commented Feb 24, 2021

View reviewed changes

remove trailing s in comment

18fe79e

noahdietz commented Feb 24, 2021

View reviewed changes

fix lint error

c60d5f1

chingor13 approved these changes Feb 25, 2021

View reviewed changes

chingor13 merged commit df55616 into googleapis:master Feb 25, 2021

This was referenced Mar 1, 2021

chore: add automerge label to code-suggester googleapis/gapic-showcase#625

Merged

fix: add labels to action.yaml #175

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support adding labels to PRs #173

feat: support adding labels to PRs #173

noahdietz commented Feb 23, 2021

codecov bot commented Feb 23, 2021 •

edited

noahdietz commented Feb 23, 2021

bcoe commented Feb 24, 2021

bcoe left a comment

noahdietz commented Feb 24, 2021

noahdietz Feb 24, 2021

noahdietz Feb 24, 2021

noahdietz Feb 24, 2021

noahdietz commented Feb 24, 2021 •

edited

noahdietz commented Feb 25, 2021

feat: support adding labels to PRs #173

feat: support adding labels to PRs #173

Conversation

noahdietz commented Feb 23, 2021

codecov bot commented Feb 23, 2021 • edited

Codecov Report

noahdietz commented Feb 23, 2021

bcoe commented Feb 24, 2021

bcoe left a comment

Choose a reason for hiding this comment

noahdietz commented Feb 24, 2021

noahdietz Feb 24, 2021

Choose a reason for hiding this comment

noahdietz Feb 24, 2021

Choose a reason for hiding this comment

noahdietz Feb 24, 2021

Choose a reason for hiding this comment

noahdietz commented Feb 24, 2021 • edited

noahdietz commented Feb 25, 2021

codecov bot commented Feb 23, 2021 •

edited

noahdietz commented Feb 24, 2021 •

edited