Skip to content

batches: support new version field #1100

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

Merged
merged 5 commits into from
Jul 10, 2024
Merged

batches: support new version field #1100

merged 5 commits into from
Jul 10, 2024

Conversation

stefanhengl
Copy link
Member

@stefanhengl stefanhengl commented Jul 9, 2024

This bumps sourcegraph/sourcegraph/lib to pull in the latest changes from https://github.com/sourcegraph/sourcegraph/pull/63613 to support the new version field for batch specs.

Notes:

  • To resolve repos, batches calls the GraphQL endpoint resolveWorkspacesForBatchSpec, which takes the serialized spec. This means all the logic to resolve repos is outsourced to Sourcegraph. This is great news for this PR because we don't have to worry how on.RepositoriesMatchingQuery is interpreted. The code path in which src-cli itself resolves the workspaces was removed in Remove src-cli workspace resolution #819.

Test plan

  • New unit tests
  • Manual testing. I tested the following workflow
    • src batch new -f test.spec.yaml (-> spec contains version: 2)
    • src batch apply -f test.spec.yaml (-> validation passes, batch spec shows up in Sourcegraph db)
    • src batch validate -f test.spec.yaml (-> validation passes or fails if I change the version to an unsupported value)
    • src batch repos -f test.spec.yaml (-> checked that returned repos match with expected pattern type)

Closes SPLF-126

if digest, err = inspectDigest(); errors.HasType(err, &fastCommandTimeoutError{}) {
if digest, err = inspectDigest(); errors.HasType[*fastCommandTimeoutError](err) {
Copy link
Member Author

Choose a reason for hiding this comment

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

errors.HasType was changed in Sourcegraph so I had to update the call here. (See https://github.com/sourcegraph/sourcegraph/pull/63024)

Comment on lines 509 to 511
const exampleSpecTmpl = `name: NAME-OF-YOUR-BATCH-CHANGE
const exampleSpecTmpl = `# "version" determines how the batch spec is interpreted.
version: 2
name: NAME-OF-YOUR-BATCH-CHANGE
Copy link
Member Author

Choose a reason for hiding this comment

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

This is returned by src batch new -f <file>

Choose a reason for hiding this comment

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

Tiny comment, as a user I would find # "version" determines how the batch spec is interpreted to be a bit mysterious. Maybe we could say

version: 2 # Use the latest schema version
name: ...

@stefanhengl stefanhengl marked this pull request as ready for review July 9, 2024 10:27
@stefanhengl stefanhengl requested a review from a team as a code owner July 9, 2024 10:27
@stefanhengl stefanhengl requested a review from a team July 9, 2024 10:27
Copy link

@jtibshirani jtibshirani left a comment

Choose a reason for hiding this comment

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

Looks great!

Comment on lines 509 to 511
const exampleSpecTmpl = `name: NAME-OF-YOUR-BATCH-CHANGE
const exampleSpecTmpl = `# "version" determines how the batch spec is interpreted.
version: 2
name: NAME-OF-YOUR-BATCH-CHANGE

Choose a reason for hiding this comment

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

Tiny comment, as a user I would find # "version" determines how the batch spec is interpreted to be a bit mysterious. Maybe we could say

version: 2 # Use the latest schema version
name: ...

Copy link
Member

@camdencheek camdencheek left a comment

Choose a reason for hiding this comment

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

Thanks!

@stefanhengl stefanhengl merged commit 3e384c4 into main Jul 10, 2024
8 checks passed
@stefanhengl stefanhengl deleted the sh/batches/version branch July 10, 2024 07:29
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.

3 participants