Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cody: Recipes PR description #51721

Merged
merged 17 commits into from
Jun 5, 2023

Conversation

deepak2431
Copy link
Contributor

@deepak2431 deepak2431 commented May 10, 2023

This PR is for issue sourcegraph/cody#183. The idea is to check the templates first with most of the possible locations of the PR template stored. If it exists, use it by summarizing the recent changes of the current checkout branch. Else, just fallback to the summarisation of commits and a checklist at the bottom.

Some of the sample PR templates generated:

iScreen Shoter - Code - 230510201407

iScreen Shoter - Code - 230510201722

Test plan

Tested it manually for different repositories with a PR template.

@cla-bot cla-bot bot added the cla-signed label May 10, 2023
@deepak2431 deepak2431 force-pushed the deepak/recipes-pr_description branch from 9334305 to 2abdfcf Compare May 10, 2023 15:04
@deepak2431
Copy link
Contributor Author

@sourcegraph/cody team, requesting a review for this PR.

@philipp-spiess philipp-spiess requested a review from a team May 17, 2023 09:38
@deepak2431
Copy link
Contributor Author

@mrnugget Made the requested changes. Also, added the changelog.

@mrnugget
Copy link
Contributor

@deepak2431 can you add a test plan to your PR description?

@deepak2431
Copy link
Contributor Author

@mrnugget I have added it.

@deepak2431 deepak2431 requested a review from mrnugget May 23, 2023 15:01
@deepak2431
Copy link
Contributor Author

@mrnugget Can you have a final look at this please?

@mrnugget
Copy link
Contributor

Looks good, let's go for it

@@ -28,6 +28,10 @@ export type RecipeID =
| 'inline-chat'
| 'next-questions'
| 'pr-description'
| 'non-stop'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These were alphabetical; let's keep them alphabetized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the fix @dominiccooney

@mrnugget mrnugget enabled auto-merge (squash) May 31, 2023 17:52
@evict evict enabled auto-merge (squash) June 1, 2023 15:27
@andreeleuterio
Copy link
Member

This PR passed Sonarcloud:
image

@deepak2431
Copy link
Contributor Author

deepak2431 commented Jun 1, 2023

@mrnugget For merging PR like this as of external contributions, there's a bit different process, as I know. The "auto-merge" doesn't work. Maybe @abeatrix or @philipp-spiess can help here.

@mrnugget
Copy link
Contributor

mrnugget commented Jun 2, 2023

Kicked off a build. Should work now.

@mrnugget
Copy link
Contributor

mrnugget commented Jun 5, 2023

@deepak2431 I don't have permission to push to your fork, can you update your fork to fix the Bazel configuration?

Ideally you run bazel configure but if you don't have bazel setup you can just update the configuration like this:

diff --git a/client/cody-shared/BUILD.bazel b/client/cody-shared/BUILD.bazel
index 3c98c06b57..6cf709008f 100644
--- a/client/cody-shared/BUILD.bazel
+++ b/client/cody-shared/BUILD.bazel
@@ -38,6 +38,7 @@ ts_project(
         "src/chat/recipes/find-code-smells.ts",
         "src/chat/recipes/fixup.ts",
         "src/chat/recipes/generate-docstring.ts",
+        "src/chat/recipes/generate-pr-description.ts",
         "src/chat/recipes/generate-release-notes.ts",
         "src/chat/recipes/generate-test.ts",
         "src/chat/recipes/git-log.ts",

@deepak2431
Copy link
Contributor Author

@mrnugget I have updated the bazel config. I hope it's fixed now.

@evict evict merged commit 16f13b1 into sourcegraph:main Jun 5, 2023
7 of 10 checks passed
ErikaRS pushed a commit that referenced this pull request Jun 22, 2023
This PR is for issue #50526. The idea is to check the templates first
with most of the possible locations of the PR template stored. If it
exists, use it by summarizing the recent changes of the current checkout
branch. Else, just fallback to the summarisation of commits and a
checklist at the bottom.

Some of the sample PR templates generated:

![iScreen Shoter - Code -
230510201407](https://github.com/sourcegraph/sourcegraph/assets/44617923/c4a50630-ad92-48e4-a50a-48b2c3f0af1a)

![iScreen Shoter - Code -
230510201722](https://github.com/sourcegraph/sourcegraph/assets/44617923/e7a92a79-5571-4cac-ad46-fee300485ffa)

## Test plan
Tested it manually for different repositories with a PR template.

---------

Co-authored-by: Thorsten Ball <mrnugget@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants