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

GitHub Actions to automate our GraphQL schema management #2108

Merged
merged 27 commits into from May 3, 2019

Conversation

Projects
2 participants
@smashwilson
Copy link
Member

commented Apr 30, 2019

Please be sure to read the contributor's guide to the GitHub package before submitting any pull requests.

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • Suggestion: You can use checklists to keep track of progress for the sections on metrics, tests, documentation, and user research.

Description of the Change

Create a GitHub Actions to manage our local copy of the GitHub GraphQL schema in the least intrusive way I can think of.

It'll fetch the latest schema from GitHub and write it into the working directory. It'll then run the relay-compiler with the updated schema. If no generated Relay files have changed, the new schema will be committed and pushed directly onto master. If there have been changes, the action will commit the schema and the Relay changes to a new branch, push it, open a pull request, and apply the "schema changed" label to it. (No pull request will be created if there's an existing open pull request with that label.)

Screenshot

N/A

Alternate Designs

We could have done this with our existing CI. Too late, this is more fun.

I had a second action I was working on that would watch as status checks and commit statuses were set on the head ref of each pull request, with the intent that it would automatically merge it if they were all green. The problem is that it would create a new check suite on every PR for each individual status that was reported, as it came in... which spammed them quite a bit.

Benefits

Ideally, this will give us an early warning for upcoming GraphQL deprecations that will cause issues for our package; the "schema-up" action will fail on its relay-compiler step if the schema has changed in an incompatible way. The action will also exclude deprecated fields, so we should see the notice before the schema change is made in production.

Possible Drawbacks

It's hard to actually validate these until they run on real webhook triggers. They might be totally broken right now. I'll just have to fix any issues as they arise?

Applicable Issues

Closes #2116, which contains all of the test runs.

Metrics

N/A

Tests

image

Documentation

I've got READMEs in the source of the ./action subdirectory.

Release Notes

N/A

User Experience Research (Optional)

N/A

smashwilson added some commits Apr 30, 2019

@smashwilson smashwilson added this to In progress in Sprint : 4 April 2019 - 8 May 2019 : v0.29.0 via automation Apr 30, 2019

@codecov

This comment has been minimized.

Copy link

commented Apr 30, 2019

Codecov Report

Merging #2108 into master will decrease coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2108      +/-   ##
==========================================
- Coverage   92.59%   92.56%   -0.04%     
==========================================
  Files         207      207              
  Lines       12036    12032       -4     
  Branches     1755     1747       -8     
==========================================
- Hits        11145    11137       -8     
- Misses        891      895       +4
Impacted Files Coverage Δ
lib/containers/user-mention-tooltip-container.js 0% <0%> (-100%) ⬇️
lib/items/issueish-detail-item.js 98.36% <0%> (-0.03%) ⬇️
lib/items/changed-file-item.js 100% <0%> (ø) ⬆️
lib/items/commit-detail-item.js 100% <0%> (ø) ⬆️
lib/items/commit-preview-item.js 100% <0%> (ø) ⬆️

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 cf10092...f6a9e52. Read the comment docs.

smashwilson added some commits Apr 30, 2019

@smashwilson smashwilson marked this pull request as ready for review Apr 30, 2019

smashwilson added some commits May 1, 2019

@smashwilson

This comment has been minimized.

Copy link
Member Author

commented May 1, 2019

I don't think there's any way for me to run this manually before we merge it into master. I'd feel better if someone other than me had a quick look at my source to find obvious bugs before I start running it, though 🙇

@smashwilson smashwilson requested a review from atom/github-package May 1, 2019

@kuychaco

This comment has been minimized.

Copy link
Member

commented May 1, 2019

Awesome, thanks so much for tackling this!

In the PR description it says:

I've got READMEs in the source of both ./action directories.

I only see one directory schema-up. I'm assuming this comment is stale?

@smashwilson

This comment has been minimized.

Copy link
Member Author

commented May 1, 2019

Hah! Yes. I originally had another action that would automatically merge the schema-change pull requests if all check suites and commit statuses were green, but it was super noisy - it added a check suite for every suite and status as it arrived. Edit'd 🙇

@kuychaco
Copy link
Member

left a comment

This looks great! Excited to get our first GitHub Action in place, and be guarded against more regressions due to graphql API deprecations!

Thanks for putting this together @smashwilson

Caught a few things that warrant attention :).

I'd be curious if we could try out testing this prior to merging by temporarily changing the Actions trigger to be on push. We can always push empty commits if we need to. That could be a more efficient way of testing, and it'd be nice if all the changes were contained in a single PR for future reference.

Show resolved Hide resolved actions/schema-up/index.js Outdated
Show resolved Hide resolved actions/schema-up/index.js Outdated
Show resolved Hide resolved actions/schema-up/index.js Outdated
Show resolved Hide resolved actions/schema-up/index.js Outdated
Show resolved Hide resolved actions/schema-up/index.js Outdated
Show resolved Hide resolved actions/schema-up/index.js

kuychaco and others added some commits May 2, 2019

Apply suggestions from code review
Co-Authored-By: smashwilson <smashwilson@github.com>

@smashwilson smashwilson referenced this pull request May 2, 2019

Closed

Schema update action tests #2116

5 of 5 tasks complete

smashwilson added some commits May 2, 2019

smashwilson added some commits May 2, 2019

git diff doesn't understand the ** splat
That might be a bash thing.
@smashwilson

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

#2116 contains all of the testing output for this, with a minimal diff on the actual Actions code:

smashwilson @ hubtop ~/src/atom/github (aw/schema-update-action-test=)
$ git diff aw/schema-update-action -- actions/ .github/
diff --git a/.github/main.workflow b/.github/main.workflow
index 490262a3b..bb567a330 100644
--- a/.github/main.workflow
+++ b/.github/main.workflow
@@ -1,6 +1,8 @@
 workflow "GraphQL schema update" {
   // Every Monday at 1am.
-  on = "schedule(0 1 * * 1)"
+  // on = "schedule(0 1 * * 1)"
+
+  on = "push"
   resolves = "Update schema"
 }

diff --git a/actions/schema-up/index.js b/actions/schema-up/index.js
index db4c34f2b..6af28f367 100644
--- a/actions/schema-up/index.js
+++ b/actions/schema-up/index.js
@@ -87,6 +87,7 @@ The GraphQL schema has been automatically updated and \`relay-compiler\` has bee
     body += relayOutput;
     body += '\n```\n\nCheck out this branch to fix things so we don\'t break.';
   }
+  tools.log.info('Pull request body:\n%s', body);

   const createPullRequestMutation = await tools.github.graphql(`
     mutation createPullRequestMutation($repositoryId: ID!, $headRefName: String!, $body: String!) {
@@ -127,6 +128,7 @@ The GraphQL schema has been automatically updated and \`relay-compiler\` has bee
   tools.exit.success(
     `Pull request #${createdPullRequest.number} has been opened and labelled for this schema upgrade.`,
   );
+  tools.exit.success('Not creating pull request to reduce testing noise');
 }, {
   secrets: ['GITHUB_TOKEN'],
 });

@smashwilson smashwilson requested a review from kuychaco May 2, 2019

@kuychaco
Copy link
Member

left a comment

Awesome work! 🚢 :shipit:

labelableId: $id,
labelIds: $labelIDs
}) {
clientMutationId

This comment has been minimized.

Copy link
@kuychaco

kuychaco May 3, 2019

Member

Hmm it's a bit annoying that you're forced to ask for return data, even if there's no use for it...

@smashwilson smashwilson merged commit 23324dd into master May 3, 2019

14 checks passed

atom.github Build #20190502.108 succeeded
Details
atom.github (Lint) Lint succeeded
Details
atom.github (Linux beta) Linux beta succeeded
Details
atom.github (Linux dev) Linux dev succeeded
Details
atom.github (Linux stable) Linux stable succeeded
Details
atom.github (MacOS beta) MacOS beta succeeded
Details
atom.github (MacOS dev) MacOS dev succeeded
Details
atom.github (MacOS stable) MacOS stable succeeded
Details
atom.github (Snapshot) Snapshot succeeded
Details
atom.github (Windows beta) Windows beta succeeded
Details
atom.github (Windows dev) Windows dev succeeded
Details
atom.github (Windows stable) Windows stable succeeded
Details
codecov/patch Coverage not affected when comparing cf10092...f6a9e52
Details
codecov/project 92.56% (-0.04%) compared to cf10092
Details

Sprint : 4 April 2019 - 8 May 2019 : v0.29.0 automation moved this from In progress to Merged May 3, 2019

@smashwilson smashwilson deleted the aw/schema-update-action branch May 3, 2019

@smashwilson

This comment has been minimized.

Copy link
Member Author

commented May 3, 2019

Thanks for the review 🙇 I guess we'll see it work live on Monday?

This was referenced May 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.