Skip to content

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

// tag don't exist locally, regardless of the format.
var digest string
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 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
@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.

4 participants