insights(sus): remove unused parameter alert

[jhchabran]

Stumbled across this while re-enabling linters. I'm not sure to fully understand the logic and as I'm focused on the linters themselves, I'd rather let you have a second look to ensure nothing fishy is happening there.

The unkeyed structs changes are unrelated, that's just me grinding through the warnings.

• TODO remove the sus prefixing this commit

Test plan

CI + careful review from @sourcegraph/code-insights

[jhchabran]

@leonore here is the suspicious thing

[leonore]

what linter did you re-enable? it's curious that this is the line it modifies when we use the same syntax for declaring errors across other calls (e.g. https://sourcegraph.com/github.com/sourcegraph/sourcegraph@7794a49876469af1922c776b48f79c1f9f713af9/-/blob/enterprise/internal/insights/resolvers/aggregates_resolvers.go?L142)

[jhchabran]

it's unparam

   enterprise/internal/insights/resolvers/aggregates_resolvers.go:236:23: `searchSuccessful` - `alert` is unused (unparam)
   func searchSuccessful(alert *search.Alert, tabulationErrors []error, shardTimeoutOccurred, runningWithExtendedTimeout, resultLimitHit bool) (bool, notAvailableReason) {
                         ^

And there are some others, which I'm putting here just in case:

   enterprise/internal/insights/resolvers/insight_view_resolvers.go:625:203: `updateCaptureGroupInsight` - `backfiller` is unused (unparam)
   func updateCaptureGroupInsight(ctx context.Context, input graphqlbackend.LineChartSearchInsightDataSeriesInput, existingSeries []types.InsightViewSeries, view types.InsightView, tx *store.InsightStore, backfiller *background.ScopedBackfiller, seriesFillStrategy fillSeriesStrategy) error {
                                                                                                                                                                                                             ^
   enterprise/internal/insights/resolvers/insight_view_resolvers.go:651:202: `updateSearchOrComputeInsight` - `backfiller` is unused (unparam)
   func updateSearchOrComputeInsight(ctx context.Context, input graphqlbackend.UpdateLineChartSearchInsightInput, existingSeries []types.InsightViewSeries, view types.InsightView, tx *store.InsightStore, backfiller *background.ScopedBackfiller, seriesFillStrategy fillSeriesStrategy) error {
                                                                                                                                                                                                            ^
   enterprise/internal/insights/resolvers/insight_view_resolvers.go:1236:167: `v1HistoricFill` - `insightEnqueuer` is unused (unparam)
   func v1HistoricFill(ctx context.Context, deprecateJustInTime bool, series types.InsightSeries, tx *store.InsightStore, scopedBackfiller *background.ScopedBackfiller, insightEnqueuer *background.InsightEnqueuer) error {
                                                                                                                                                                         ^
   enterprise/internal/insights/resolvers/insight_view_resolvers.go:1252:195: createAndAttachSeries - result 0 (*github.com/sourcegraph/sourcegraph/enterprise/internal/insights/types.InsightSeries) is never used (unparam)
   func createAndAttachSeries(ctx context.Context, tx *store.InsightStore, startSeriesFill fillSeriesStrategy, view types.InsightView, series graphqlbackend.LineChartSearchInsightDataSeriesInput) (*types.InsightSeries, error) {

[leonore]

ty! I know the logic behind most of these and can resolve those issues, just want to confirm with @chwarwick why we don't assert on the alert in searchSuccessful?

[chwarwick]

@leonore I believe the only alert at the time was the timeout, and in general because alerts come with results we decided it was better to show what results we got than to show an error unlike in a the other insight case where we want to hold off and ensure there was a full exhaustive result.

[leonore]

ok, in that case I'll just remove the parameter from the function then

[leonore]

the PR already removes it actually, @chwarwick since half of this PR is now my code can you review and approve if it looks good?

[jhchabran]

I'd appreciate it if we could go quick on this, as I'll get blocked in merging the upcoming PR for linters if this sticks around too long :)

[jhchabran]

👀 #45548

[sourcegraph-bot]

Codenotify: Notifying subscribers in CODENOTIFY files for diff 7794a49...8e21ab1.

Notify	File(s)
@sourcegraph/code-insights-backend	enterprise/internal/insights/resolvers/aggregates_resolvers.go enterprise/internal/insights/resolvers/insight_view_resolvers.go enterprise/internal/insights/resolvers/insight_view_resolvers_test.go enterprise/internal/insights/store/store_test.go