Skip to content

Enable gocritic offBy1 and fix import warning slicing (codex)#3713

Merged
denik merged 2 commits intodatabricks:mainfrom
denik:codex/investigate-and-recommend-golangci-lint-linters
Oct 3, 2025
Merged

Enable gocritic offBy1 and fix import warning slicing (codex)#3713
denik merged 2 commits intodatabricks:mainfrom
denik:codex/investigate-and-recommend-golangci-lint-linters

Conversation

@denik
Copy link
Contributor

@denik denik commented Oct 3, 2025

Changes

  • enable the gocritic offBy1 check in golangci-lint so we catch missing Index() bounds handling
  • guard the Terraform import warning truncation against missing "Warning:" blocks to avoid slice panics

https://chatgpt.com/s/cd_68df8308cdf081918babe8e111fb5dce

@github-actions
Copy link

github-actions bot commented Oct 3, 2025

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 3713
  • Commit SHA: 99f816a87f85ed5cacae9e187a04f5fe06a5f63d

Checks will be approved automatically on success.

// Remove output starting from Warning until end of output, if present.
if idx := strings.Index(output, "Warning:"); idx != -1 {
output = output[:idx]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated to the change at hand, but this should really use https://pkg.go.dev/strings#CutPrefix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CutPrefix would remove prefix, but this removes suffix :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, lol!

@denik denik enabled auto-merge October 3, 2025 09:33
@eng-dev-ecosystem-bot
Copy link
Collaborator

eng-dev-ecosystem-bot commented Oct 3, 2025

Run: 18220837735

Env ✅​pass 🔄​flaky 🙈​skip
✅​ aws linux 320 537
✅​ aws windows 321 536
🔄​ aws-ucws linux 427 9 433
🔄​ aws-ucws windows 428 9 432
✅​ azure linux 320 536
✅​ azure windows 321 535
🔄​ azure-ucws linux 427 9 432
🔄​ azure-ucws windows 430 7 431
✅​ gcp linux 319 538
✅​ gcp windows 320 537
9 failing tests:
Test Name aws-ucws linux aws-ucws windows azure-ucws linux azure-ucws windows
TestAccept/bundle/deploy/dashboard/detect-change 🔄​flaky 🔄​flaky 🔄​flaky 🔄​flaky
TestAccept/bundle/deploy/dashboard/generate_inplace 🔄​flaky 🔄​flaky 🔄​flaky 🔄​flaky
TestAccept/bundle/deploy/dashboard/nested-folders 🔄​flaky 🔄​flaky 🔄​flaky 🔄​flaky
TestAccept/bundle/deploy/dashboard/simple 🔄​flaky 🔄​flaky 🔄​flaky 🔄​flaky
TestAccept/bundle/deploy/dashboard/simple_outside_bundle_root 🔄​flaky 🔄​flaky 🔄​flaky 🔄​flaky
TestAccept/bundle/deploy/dashboard/simple_syncroot 🔄​flaky 🔄​flaky 🔄​flaky 🔄​flaky
TestAccept/bundle/deployment/bind/dashboard 🔄​flaky 🔄​flaky 🔄​flaky ✅​pass
TestAccept/bundle/deployment/bind/dashboard/recreation 🔄​flaky 🔄​flaky 🔄​flaky ✅​pass
TestDashboardAssumptions_WorkspaceImport 🔄​flaky 🔄​flaky 🔄​flaky 🔄​flaky

@denik denik disabled auto-merge October 3, 2025 11:32
@denik denik merged commit acead37 into databricks:main Oct 3, 2025
12 of 13 checks passed
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

Comments