Skip to content

Comments

chore: remove legacy deps, deduplicate tests, update infra#220

Open
olegshmuelov wants to merge 6 commits intofeat/compounding-supportfrom
chore/codebase-health
Open

chore: remove legacy deps, deduplicate tests, update infra#220
olegshmuelov wants to merge 6 commits intofeat/compounding-supportfrom
chore/codebase-health

Conversation

@olegshmuelov
Copy link

@olegshmuelov olegshmuelov commented Feb 15, 2026

Summary

  • Fix Ubuntu-specific CA bundle path (affects production deployments)
  • Deduplicate integration test setup, remove unused code
  • Replace go-tasks.yml with modern CI (ci.yml) and update releases.yml
  • Add .golangci.yml v2 config
  • Clean up .gitignore, Dockerfile, Makefile, regenerate TLS test certs

Test plan

  • CI passes (build, unit tests, integration tests, lint)

@olegshmuelov olegshmuelov marked this pull request as ready for review February 17, 2026 14:58
@olegshmuelov olegshmuelov requested a review from y0sher February 18, 2026 08:54
@momosh-ssv
Copy link

Can we please split this unit into multiple PRs? I think we have 5 distinct concerns here, review is really hard and I worry that we're entering some rollback risky waters here.

Proposed split: 5 focused PRs

PR A — fix: use system CA bundle by default

  • Scope: cli/flags/init.go, cli/flags/reshare.go, cli/flags/resign.go
  • Why first: This is a real bug fix with user-visible impact. It's tiny, self-contained, and easy to test. Gets the fix shipped fast.

PR B — chore: replace bloxapp/ssv/logging with direct zap

  • Scope: cli/utils/utils.go, cli/initiator/initiator_health_check.go, go.mod, go.sum
  • Why separate: Dependency changes deserve their own PR, so we can audit the go.mod diff in isolation and verify no regressions in logging behavior.

PR C — chore: update CI/CD and linting infrastructure

  • Scope: .github/workflows/ci.yml (new), .github/workflows/go-tasks.yml (deleted), .github/workflows/releases.yml, .golangci.yml, Makefile, Dockerfile, docker-compose.yml, entry-point.sh
  • Why separate: CI/infra changes are orthogonal to code changes. Reviewers with DevOps context can focus here without needing to understand Go code changes.

PR D — chore: deduplicate integration test setup

  • Scope: All integration_test/*.go files, pkgs/utils/test_utils/test_utls.go, integration_test/certs/
  • Why separate: Pure test refactor with no production code changes. We can verify test behavior is preserved without worrying about functional impact.

PR E — chore: housekeeping and minor code cleanups

  • Scope: .gitignore, CLAUDE.md, README.md, cmd/ssv-dkg/ssv-dkg.go, cli/flags/utils.go, cli/flags/verify.go, cli/initiator/reshare.go, cli/verify/verify.go, pkgs/crypto/, pkgs/dkg/, pkgs/initiator/, pkgs/operator/, pkgs/wire/, deleted example .csr/.srl files
  • Why last: Pure housekeeping — no logic changes. Easy to rubber-stamp once the substantive PRs are merged.

Recommended Merge Order

  • A before B: B touches go.mod; A is pure flag logic, no dependency overlap.
  • B before C: CI should test the new logging code.
  • C before D: Integration tests run in CI; have CI ready before refactoring tests.
  • D before E: Test dedup may touch some of the same files as E's cleanups.

@olegshmuelov
Copy link
Author

olegshmuelov commented Feb 23, 2026

Can we please split this unit into multiple PRs? I think we have 5 distinct concerns here, review is really hard and I worry that we're entering some rollback risky waters here.

Thanks for the detailed breakdown.

The challenge with splitting is that key changes span multiple of the proposed PRs.
For example, bloxapp/ssv is imported in cli/utils/ (logging - your PR B), integration tests (logging - your PR D), and pkgs/crypto/ (rsaencryption - your PR E).
The dependency can only be removed from go.mod once all three are merged, so PR B would be an incomplete cleanup that ships a half-removed dep.

In terms of risk - there are no production logic changes here. The scope is:

  • Infra (CI, Dockerfile, Makefile) - easily verified by CI passing
  • Lint - stricter checks that catch issues earlier
  • Test dedup - same scenarios, table-driven instead of copy-paste
  • Dependency removal - replacing bloxapp/ssv imports with direct equivalents

I'd prefer not to prolong this as we have downstream work (0x02 withdrawal credentials) blocked on this branch. That said, happy to walk you through any section or address any specific concerns you raise.

@momosh-ssv
Copy link

In terms of risk - there are no production logic changes here.

Right, I'll be focusing on logic part then

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.

2 participants