Skip to content

batches: fix feature detection in src batch validate#577

Merged
LawnGnome merged 1 commit into
mainfrom
aharvey/validate-against-features
Aug 12, 2021
Merged

batches: fix feature detection in src batch validate#577
LawnGnome merged 1 commit into
mainfrom
aharvey/validate-against-features

Conversation

@LawnGnome

Copy link
Copy Markdown
Contributor

When creating the Service instance, src batch validate failed to request or initialise the Sourcegraph feature flags based on the Sourcegraph version in use. As a result, only baseline features were enabled when parsing and validating batch specs.

Fixes #576.

When creating the `Service` instance, `src batch validate` failed to
request or initialise the Sourcegraph feature flags based on the
Sourcegraph version in use. As a result, only baseline features were
enabled when parsing and validating batch specs.

Fixes #576.
@LawnGnome LawnGnome requested a review from a team August 12, 2021 22:45

@eseliger eseliger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great catch!

Comment thread cmd/src/batch_validate.go
Comment on lines +44 to +50
svc := service.New(&service.Opts{
Client: cfg.apiClient(apiFlags, flagSet.Output()),
})

if err := svc.DetermineFeatureFlags(ctx); err != nil {
return err
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a side-note, no action required: It'd be nice if this is not something that can be forgotten 😬

package service

func NewWithFeatureFlags(client *api.Client) (Service, error) {
	svc := service.New(&service.Opts{
		Client: client,
	})
	err := svc.DetermineFeatureFlags(ctx)
	return svc, err
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was on the fence on this one too. I think I'd rather keep this PR as trivial as it is for now, but we might want to rethink in general how we design and couple the service at some point, since it's not as simple as it once was.

@LawnGnome LawnGnome merged commit 7ee2619 into main Aug 12, 2021
@LawnGnome LawnGnome deleted the aharvey/validate-against-features branch August 12, 2021 23:29
scjohns pushed a commit that referenced this pull request Apr 24, 2023
When creating the `Service` instance, `src batch validate` failed to
request or initialise the Sourcegraph feature flags based on the
Sourcegraph version in use. As a result, only baseline features were
enabled when parsing and validating batch specs.

Fixes #576.
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.

batches: feature detection fails in src batch validate

2 participants