batches: `supportsCommitSigning ` field to `BatchChangesCodeHosts` by st0nebreaker · Pull Request #52473 · sourcegraph/sourcegraph-public-snapshot

st0nebreaker · 2023-05-25T22:37:43Z

Closes #52465, setup for #52220

Adds new supportsCommitSigning field to the BatchChangesCodeHosts resolver. Only GitHub external services have commit signing for now. Later on we will support more code hosts. Even though different mechanisms will support the commit signing (ie SSH keys), that will be handled downstream.

Test plan

Nav to /site-admin/batch-changes
Ensure /graphql?GlobalBatchChangesCodeHosts returns the correct value for supportsCommitSigning

sourcegraph-bot · 2023-05-25T22:44:04Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff 98bc90f...8dc039f.

Notify	File(s)
@courier-new	cmd/frontend/graphqlbackend/batches.go
@eseliger	cmd/frontend/graphqlbackend/batches.go enterprise/internal/batches/types/code_host.go

sourcegraph-buildkite · 2023-05-25T22:51:48Z

Bundle size report 📦

Initial size	Total size	Async size	Modules
0.00% (0.00 kb)	0.00% (+0.01 kb)	0.00% (+0.01 kb)	0.00% (0)

Look at the Statoscope report for a full comparison between the commits 8dc039f and 60d130d or learn more.

Open explanation

Initial size is the size of the initial bundle (the one that is loaded when you open the page)
Total size is the size of the initial bundle + all the async loaded chunks
Async size is the size of all the async loaded chunks
Modules is the number of modules in the initial bundle

sourcegraph-bot · 2023-05-25T22:52:26Z

📖 Storybook live preview

courier-new

Code looks fine, but I have a bit of feedback regarding intentions for adding this field! I admit on first read, I thought hasCommitSigning had to do with whether or not a code host had a GH App already configured for it for commit signing, but it's clear to me now from your PR description and changes that it's actually referring to whether or not a code host type can support commit signing today. With that in mind, two questions:

Could we name the field supportsCommitSigning instead, to clear up what it refers to?
Do you mind just briefly walking me through how we intend to use this field on the frontend? I could make some guesses, and I may have some feedback based on those guesses, but I'd rather hear from you (or Bolaji) first. 🙂

st0nebreaker · 2023-05-25T23:13:26Z

Yes I can definitely rename it. I was aiming to match the HasWebhooks language, but "supports" here is more clear - agreed.

Do you mind just briefly walking me through how we intend to use this field on the frontend? I could make some guesses, and I may have some feedback based on those guesses, but I'd rather hear from you (or Bolaji) first. 🙂

Sure thing - The train of thought is because we will allow a GH App for each GH instance, we'll show each code host connection, this keeps the same format as our code host tokens too for uniformity. The frontend will read if supportsCommitSigning = true to give any GH App actions or show some disabled/not-supported badge state.

Later on, say if we support commit signing through SSH keys for GitLab/BitBucket, we can determine on the frontend node whether to show Create GH App or Add credentials button. Does that capture the thinking @BolajiOlajide ?

Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>

courier-new

Got it, thanks for that explanation. I was thinking about alternative ways to surface this information but ultimately I think I like the flexibility that this simple boolean property affords us for the future. To reiterate what I said in Slack in a more public forum, I like that this field is completely agnostic to the actual underlying method we use to sign commits. On top of that, I was thinking about how we'd practically pair this property on the frontend with some sort of joint query for the GH App, and I settled on this proposal for that, which I think pairs nicely with this property. 🙂 So 👍

courier-new · 2023-05-26T03:07:35Z

        externalServiceURL
        requiresSSH
        requiresUsername
+        supportsCommitSigning


Thank you for validating the property appears in the network request as part of your test plan! For now, it might make more sense to drop it and only push the backend changes in this PR. Otherwise, the TS compiler starts to expect this property to be required everywhere we're mocking a code host (e.g. in storybook stories) and grows quite angry when it's not there. 😬 I don't want you to have to bloat this PR with all those mock data changes!

Oh good call. Yea I'll leave that bloat for the FE consumption of this PR 😁 Removed ✅

…52473) Closes #[52465](https://github.com/sourcegraph/sourcegraph/issues/52465), setup for #[52220](https://github.com/sourcegraph/sourcegraph/issues/52220) Adds new `supportsCommitSigning ` field to the `BatchChangesCodeHosts` resolver. Only GitHub external services have commit signing for now. Later on we will support more code hosts. Even though different mechanisms will support the commit signing (ie SSH keys), that will be handled downstream. ## Test plan ![image](https://github.com/sourcegraph/sourcegraph/assets/59381432/dd77362f-769e-4f5d-a8aa-ae755344e9fc) - Nav to `/site-admin/batch-changes` - Ensure `/graphql?GlobalBatchChangesCodeHosts` returns the correct value for `supportsCommitSigning ` --------- Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>

New hasCommitSigning property on BatchChangesCodeHost

f9ff2c7

cla-bot Bot added the cla-signed label May 25, 2023

st0nebreaker changed the base branch from main to batches/commit-signing May 25, 2023 22:38

st0nebreaker commented May 25, 2023

View reviewed changes

Comment thread enterprise/cmd/frontend/internal/batches/resolvers/code_host.go Outdated

st0nebreaker marked this pull request as ready for review May 25, 2023 22:42

st0nebreaker requested a review from a team May 25, 2023 22:42

st0nebreaker self-assigned this May 25, 2023

st0nebreaker added the batches-signed-commit label May 25, 2023

sourcegraph-bot-devx mentioned this pull request May 25, 2023

Sign commits created by Batch Changes with GitHub App (Tracking) #48611

Closed

31 tasks

courier-new reviewed May 25, 2023

View reviewed changes

Comment thread enterprise/cmd/frontend/internal/batches/resolvers/code_host.go Outdated

Comment thread enterprise/cmd/frontend/internal/batches/resolvers/code_host.go Outdated

st0nebreaker and others added 2 commits May 25, 2023 17:15

Update enterprise/cmd/frontend/internal/batches/resolvers/code_host.go

8e50c7e

Co-authored-by: Kelli Rockwell <kelli@sourcegraph.com>

Rename field

7008883

courier-new approved these changes May 26, 2023

View reviewed changes

st0nebreaker changed the title ~~batches: hasCommitSigning field to BatchChangesCodeHosts~~ batches: supportsCommitSigning field to BatchChangesCodeHosts May 26, 2023

Rm FE property in query

8dc039f

st0nebreaker merged this pull request into batches/commit-signing May 26, 2023

st0nebreaker deleted the becca/commitSigningEnabled branch May 26, 2023 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

batches: `supportsCommitSigning` field to `BatchChangesCodeHosts`#52473

batches: `supportsCommitSigning` field to `BatchChangesCodeHosts`#52473
st0nebreaker merged 4 commits into
batches/commit-signingfrom
becca/commitSigningEnabled

st0nebreaker commented May 25, 2023 •

edited

Loading

Uh oh!

Uh oh!

sourcegraph-bot commented May 25, 2023 •

edited

Loading

Uh oh!

sourcegraph-buildkite commented May 25, 2023 •

edited

Loading

Uh oh!

sourcegraph-bot commented May 25, 2023 •

edited

Loading

Uh oh!

courier-new left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

st0nebreaker commented May 25, 2023

Uh oh!

courier-new left a comment

Uh oh!

courier-new May 26, 2023 •

edited

Loading

Uh oh!

st0nebreaker May 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

st0nebreaker commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test plan

Uh oh!

Uh oh!

sourcegraph-bot commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sourcegraph-buildkite commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bundle size report 📦

Uh oh!

sourcegraph-bot commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

courier-new left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

st0nebreaker commented May 25, 2023

Uh oh!

courier-new left a comment

Choose a reason for hiding this comment

Uh oh!

courier-new May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

st0nebreaker May 26, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

st0nebreaker commented May 25, 2023 •

edited

Loading

sourcegraph-bot commented May 25, 2023 •

edited

Loading

sourcegraph-buildkite commented May 25, 2023 •

edited

Loading

sourcegraph-bot commented May 25, 2023 •

edited

Loading

courier-new left a comment •

edited

Loading

courier-new May 26, 2023 •

edited

Loading