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

Switch over to full GraphQL #377

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from
Draft

Switch over to full GraphQL #377

wants to merge 11 commits into from

Conversation

lilyminium
Copy link
Contributor

@lilyminium lilyminium commented Sep 11, 2024

Fixes #370, I think.

This PR stops using PyGithub entirely and switches over to using GraphQL directly. It's not strictly necessary since the only issue with PyGithub was to do with projects, but as the project card objects are passed into every state-update function, I only realized this after rewriting the whole thing. The main overhead added is that we now use the GraphQL API to add/remove labels, get files, and add comments. If this is too much maintenance overhead I can try to smush together this PR and #375.

The new code is in lifecycle-v2.py. I added a test CI that runs through some steps (e.g. adding/removing labels, adding comments, moving cards around) that should only trigger on PRs labelled with test-ci. That's what's repsonsible for the mess here and in #375.

@lilyminium lilyminium closed this Sep 11, 2024
@lilyminium lilyminium reopened this Sep 11, 2024
@openff-dangerbot openff-dangerbot added test-label This is a test label to be added or removed automatically via CI. and removed test-label This is a test label to be added or removed automatically via CI. labels Sep 11, 2024
@openff-dangerbot
Copy link
Contributor

This is a test comment from running CI. Please ignore

@openff-dangerbot openff-dangerbot added test-label This is a test label to be added or removed automatically via CI. and removed test-label This is a test label to be added or removed automatically via CI. labels Sep 11, 2024
@openff-dangerbot
Copy link
Contributor

This is a test comment from running CI. Please ignore

@openff-dangerbot openff-dangerbot added test-label This is a test label to be added or removed automatically via CI. and removed test-label This is a test label to be added or removed automatically via CI. labels Sep 11, 2024
@openff-dangerbot
Copy link
Contributor

This is a test comment from running CI. Please ignore

@openff-dangerbot openff-dangerbot added test-label This is a test label to be added or removed automatically via CI. and removed test-label This is a test label to be added or removed automatically via CI. labels Sep 11, 2024
@openff-dangerbot
Copy link
Contributor

This is a test comment from running CI. Please ignore

@openff-dangerbot openff-dangerbot added test-label This is a test label to be added or removed automatically via CI. and removed test-label This is a test label to be added or removed automatically via CI. labels Sep 11, 2024
@openff-dangerbot
Copy link
Contributor

This is a test comment from running CI. Please ignore

Copy link
Member

@j-wags j-wags left a comment

Choose a reason for hiding this comment

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

Thanks for taking a stab at this. I guess I see two paths forward here:

  • Fix this ourselves in the short term, with a mix of manual intervention and direct graphql-based automation
  • Wait for pygithub to update to the new project board interface

Given that this repo has no (/few?) tests, that replacing this functionality will add a lot of lines, and I don't know graphql beyond some stuff I hacked together in my last PR, I don't think I can meaningfully review this PR. But on the other hand, given that this only has internal consumers, that our previous QCA datasets are immutable, and that we can always roll things back (or wait for the pygithub update), I think it's fine to move ahead with this in the short term.

So feel free to merge this when you think it's good, and even merge things without my review/commit directly to master to avoid having to wait for my responses :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Backlog
Development

Successfully merging this pull request may close these issues.

Update automation for new GH project boards
3 participants