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

Add mergebot #1023

Merged
merged 16 commits into from
Jul 1, 2024
Merged

Add mergebot #1023

merged 16 commits into from
Jul 1, 2024

Conversation

sandersn
Copy link
Member

@sandersn sandersn commented Jun 27, 2024

Initial commit only so far. I made a list of tasks that I think need to happen and I'll ask for help vetting it.

  • Update .knip settings with the actual entrypoint(s).
  • Make sure name change from dt-mergebot to mergebot happens inside the copied files.
  • copy the deploy on push PR (and update paths now that it's in dt-tools)
  • copy the daily PR sync workflow
  • make sure workflow auth tokens are available.
  • copy the test workflow
  • make sure lint uses the repo's lint and format
  • update readme on dt-mergebot to point to this location (like dts-gen, dts-critic, etc)

Later:

  • Update mergebot tsconfig to use ../tsconfig-base
  • Set up base test tsconfig.
  • update dependencies
  • figure out generated intermediate types, maybe by defining fragments?

@@ -158,7 +158,7 @@ function doRestCall(call: RestMutation): Promise<void> {
const url = `https://api.github.com/repos/DefinitelyTyped/DefinitelyTyped/${call.op}`;
const headers = {
"accept": "application/vnd.github.v3+json",
"authorization": `token ${process.env.BOT_AUTH_TOKEN}`,
Copy link
Member

Choose a reason for hiding this comment

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

I would not recommend changing any of the token names; we have these set in the function app. If anything I would rather us become consistent and just do GITHUB_TOKEN like most other tools.

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, right. I got confused between the variables available to workflows vs functions.

@@ -0,0 +1,39 @@
name: CI
Copy link
Member

Choose a reason for hiding this comment

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

Long term I don't think we need this; we already have a workflow which does testing and should be doing these tests.

environment:
name: 'Production'
permissions:
id-token: write
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
id-token: write
contents: read
id-token: write

run: TYPESCRIPT_BOT_TOKEN=secret node packages/mergebot/dist/functions/index.js

- name: Create zip
run: zip -r ${{ env.FUNCTION_ZIP_NAME }} packagaes/mergebot/dist packages/mergebot/host.json packages/mergebot/package.json
Copy link
Member

@jakebailey jakebailey Jun 28, 2024

Choose a reason for hiding this comment

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

Will need to check this; the paths put into the zip may now be nested because we're using longer paths as the input. Better would be to consistently use working-directory, then output to ${{ env.GITHUB_WORKSPACE }}/${{ env.FUNCTION_ZIP_NAME }}.

sandersn added 11 commits June 28, 2024 11:42
Restore to version from main, then install using the local version of
pnpm.
1. Name conflict from changing TOO_MANY_FILES to tooManyFiles. Renamed
again.
2. Placeholder tsconfig for mergebot test doesn't exist. There is no
separate tconfig for tests!
Add entrypoints, ignore graphql, remove unused devDeps
1. Add test.yml steps to ci.yml.
2. Some fixes to deploy.yml.
2. jest now runs mergebot tests
No error on my machine, but maybe it'll help CI
@sandersn sandersn merged commit 97abc80 into microsoft:main Jul 1, 2024
7 checks passed
@sandersn sandersn deleted the add-mergebot branch July 1, 2024 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants