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

CI Pipeline: create cassettes in fork #4257

Conversation

waynehamadi
Copy link
Contributor

@waynehamadi waynehamadi commented May 16, 2023

Background

This is a hotfix for our current cassette workflow.
Current process:

  • if prompt change
  • create branch elsewhere with cassettes
  • when we merge, it creates a PR that needs to be merged

New process:

  • prompt changes
  • create branch and create a PR whose target is the fork
  • the user merges the pr to have the pipeline passing

Changes

Documentation

Test Plan

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

@waynehamadi waynehamadi requested a review from a team May 16, 2023 23:13
@vercel
Copy link

vercel bot commented May 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) May 17, 2023 2:44am

@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (52874cc) 62.75% compared to head (ec8eebb) 62.75%.

❗ Current head ec8eebb differs from pull request most recent head a75bc71. Consider uploading reports for the commit a75bc71 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4257   +/-   ##
=======================================
  Coverage   62.75%   62.75%           
=======================================
  Files          73       73           
  Lines        3367     3367           
  Branches      487      487           
=======================================
  Hits         2113     2113           
  Misses       1107     1107           
  Partials      147      147           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@waynehamadi waynehamadi force-pushed the feature/create-cassette-pr-in-fork branch from 771ddab to e8d5085 Compare May 16, 2023 23:17
@waynehamadi waynehamadi marked this pull request as draft May 16, 2023 23:17
@waynehamadi waynehamadi force-pushed the feature/create-cassette-pr-in-fork branch from e8d5085 to cdf487c Compare May 16, 2023 23:22
@github-actions github-actions bot added size/xl and removed size/l labels May 16, 2023
@github-actions
Copy link
Contributor

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@waynehamadi waynehamadi force-pushed the feature/create-cassette-pr-in-fork branch from 857faec to fb17f13 Compare May 16, 2023 23:35
@github-actions
Copy link
Contributor

This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size

@waynehamadi waynehamadi force-pushed the feature/create-cassette-pr-in-fork branch from dd48514 to ec8eebb Compare May 16, 2023 23:53
@github-actions github-actions bot added size/l and removed size/xl labels May 16, 2023
@dschonholtz
Copy link
Contributor

I’m gonna start being a bit more insistent about a couple things.
go through the check boxes. Think about them tick them as appropriate.
Fill in the different sections of the pr template.

What is the GitHub action you are using? This is about security so we want to make sure we aren’t exposing ourselves to a pr bot someone else owns that will exfiltrate keys or something.

You have asked for multiple reviews so don’t leave this as a draft PR. That’s confusing.

What’s the test plan. I’m assuming that the plan is to just keep putting in hot fixes until the pipeline works and cassettes run, but that is exactly that. It is an assumption

Technical debt grows and feeds on itself it is worth the time on individual PRs

@waynehamadi waynehamadi marked this pull request as ready for review May 17, 2023 01:50
@waynehamadi
Copy link
Contributor Author

@dschonholtz alright I will do that, you have some comments about the actual change ?

@ntindle ntindle self-assigned this May 17, 2023
@ntindle ntindle merged commit 55af3e1 into Significant-Gravitas:master May 17, 2023
@waynehamadi
Copy link
Contributor Author

@dschonholtz oh it got merged, but here is the test plan:
now I need to create a pr to see if the cassette are created against the forks, if it doesn't work I will debug it
The github actions are pretty common for both. But only our github token is at risk and it changes at every workflow

ppetermann pushed a commit to ppetermann/Auto-GPT that referenced this pull request May 22, 2023
@ntindle ntindle added this to the v0.4.0 Release milestone May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants