Skip to content

Commit 03fea65

Browse files
committed
feat(ci): add lint, tidy, and formatting CI jobs
Split the monolithic build-and-unit-test job into three parallel jobs: - lint: gofmt, yamllint, jsonlint, shellcheck - tidy: go mod tidy, bazel mod tidy, gazelle - build-and-unit-test: bazel build + unit tests Add corresponding Makefile targets (check-fmt, check-gazelle, check-json, check-shell, check-tidy, check-yaml, fmt, lint) so CI and local dev use the same validation logic.
1 parent 5aa1dae commit 03fea65

22 files changed

Lines changed: 196 additions & 90 deletions

File tree

.github/workflows/ci.yml

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,29 +17,57 @@ permissions:
1717

1818
jobs:
1919
# ---------------------------------------------------------------------------
20-
# BUILD AND UNIT TESTS (special case - Gazelle + build + unit tests)
20+
# LINT (gofmt from Bazel Go SDK + yamllint + shellcheck + jq)
2121
# ---------------------------------------------------------------------------
22-
build-and-unit-test:
23-
name: Build and Unit Test
22+
lint:
23+
name: Lint
24+
runs-on: ubuntu-latest
25+
steps:
26+
- uses: actions/checkout@v4
27+
- uses: ./.github/actions/setup
28+
29+
- name: Set up Python
30+
uses: actions/setup-python@v5
31+
with:
32+
python-version: '3.x'
33+
34+
- name: Install linters
35+
run: pip install yamllint
36+
37+
- name: Run linters
38+
run: make lint
39+
40+
# ---------------------------------------------------------------------------
41+
# TIDY (module files + BUILD files in sync)
42+
# ---------------------------------------------------------------------------
43+
tidy:
44+
name: Tidy
2445
runs-on: ubuntu-latest
2546
steps:
2647
- uses: actions/checkout@v4
2748
- uses: ./.github/actions/setup
2849

50+
- name: Set up Go
51+
uses: actions/setup-go@v5
52+
with:
53+
go-version-file: 'go.mod'
54+
cache: false
55+
56+
- name: Check module files are tidy
57+
run: make check-tidy
58+
2959
- name: Check BUILD files are up to date
30-
run: |
31-
echo "Running Gazelle to check BUILD files..." >&2
32-
make gazelle
33-
if ! git diff --quiet; then
34-
echo "BUILD files are out of date!" >&2
35-
echo "" >&2
36-
echo "The following files were modified by Gazelle:" >&2
37-
git diff --name-only >&2
38-
echo "" >&2
39-
echo "Please run 'make gazelle' locally and commit the changes." >&2
40-
exit 1
41-
fi
42-
echo "BUILD files are up to date" >&2
60+
run: make check-gazelle
61+
62+
# ---------------------------------------------------------------------------
63+
# BUILD AND UNIT TESTS
64+
# ---------------------------------------------------------------------------
65+
build-and-unit-test:
66+
name: Build and Unit Test
67+
runs-on: ubuntu-latest
68+
steps:
69+
- uses: actions/checkout@v4
70+
- uses: ./.github/actions/setup
4371

4472
- name: Build project
4573
run: make build
@@ -120,6 +148,8 @@ jobs:
120148
name: Required Checks
121149
runs-on: ubuntu-latest
122150
needs:
151+
- lint
152+
- tidy
123153
- build-and-unit-test
124154
- e2e
125155
- gateway-integration-test

.yamllint.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
extends: default
2+
3+
rules:
4+
document-start: disable
5+
line-length: disable
6+
truthy:
7+
check-keys: false

Makefile

Lines changed: 78 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ LOCAL_PROJECT = submitqueue
1313
# Set REPO_ROOT for docker-compose
1414
export REPO_ROOT := $(shell pwd)
1515

16-
.PHONY: build build-all-linux build-gateway-linux build-orchestrator-linux clean clean-proto deps e2e-test gazelle integration-test integration-test-extensions integration-test-gateway integration-test-orchestrator local-clean local-gateway-start local-gateway-stop local-init-schemas local-logs local-orchestrator-start local-orchestrator-stop local-ps local-restart local-start local-stop proto query-deps query-targets run-client-gateway run-client-orchestrator run-queue-admin test test-no-cache help
16+
.PHONY: build build-all-linux build-gateway-linux build-orchestrator-linux check-gazelle check-tidy clean clean-proto deps e2e-test fmt gazelle integration-test integration-test-extensions integration-test-gateway integration-test-orchestrator lint lint-fmt lint-json lint-shell lint-yaml local-clean local-gateway-start local-gateway-stop local-init-schemas local-logs local-orchestrator-start local-orchestrator-stop local-ps local-restart local-start local-stop proto query-deps query-targets run-client-gateway run-client-orchestrator run-queue-admin test test-no-cache help
1717

1818

1919
build: ## Build all services and examples
@@ -41,6 +41,36 @@ build-orchestrator-linux: ## Build Orchestrator Linux binary for Docker
4141
cp -f bazel-bin/example/server/orchestrator/orchestrator .docker-bin/orchestrator
4242
@echo "Orchestrator Linux binary ready at .docker-bin/orchestrator"
4343

44+
check-gazelle: ## Check BUILD.bazel files are up to date
45+
@echo "Running Gazelle to check BUILD files..."
46+
@$(BAZEL) run //:gazelle
47+
@if ! git diff --quiet; then \
48+
echo "BUILD files are out of date!" >&2; \
49+
echo "" >&2; \
50+
echo "The following files were modified by Gazelle:" >&2; \
51+
git diff --name-only >&2; \
52+
echo "" >&2; \
53+
echo "Please run 'make gazelle' locally and commit the changes." >&2; \
54+
exit 1; \
55+
fi
56+
@echo "BUILD files are up to date."
57+
58+
check-tidy: ## Check that go.mod and MODULE.bazel are tidy
59+
@echo "Running go mod tidy..."
60+
@go mod tidy -e
61+
@echo "Running bazel mod tidy..."
62+
@$(BAZEL) mod tidy
63+
@if ! git diff --quiet; then \
64+
echo "Module files are out of date!" >&2; \
65+
echo "" >&2; \
66+
echo "The following files were modified:" >&2; \
67+
git diff --name-only >&2; \
68+
echo "" >&2; \
69+
echo "Please run 'go mod tidy' and 'bazel mod tidy' locally and commit the changes." >&2; \
70+
exit 1; \
71+
fi
72+
@echo "Module files are up to date."
73+
4474
clean: ## Clean generated files and binaries
4575
@echo "Cleaning with Bazel..."
4676
@$(BAZEL) clean
@@ -63,6 +93,12 @@ e2e-test: build-all-linux ## Run end-to-end tests (hermetic, auto-builds binarie
6393
@echo "Running end-to-end tests..."
6494
@$(BAZEL) test //test/e2e:e2e_test --test_output=streamed
6595

96+
fmt: ## Format Go code
97+
@GOFMT=$$($(BAZEL) run @rules_go//go -- env GOROOT 2>/dev/null | tail -1)/bin/gofmt; \
98+
echo "Formatting Go code (using $$GOFMT)..."; \
99+
$$GOFMT -w .; \
100+
echo "Formatting complete!"
101+
66102
gazelle: ## Update BUILD.bazel files
67103
@echo "Running Gazelle to update BUILD files..."
68104
@$(BAZEL) run //:gazelle
@@ -83,6 +119,47 @@ integration-test-orchestrator: build-orchestrator-linux ## Run Orchestrator inte
83119
@echo "Running Orchestrator integration tests..."
84120
@$(BAZEL) test //test/integration/orchestrator:orchestrator_test --test_output=streamed
85121

122+
lint: lint-fmt lint-json lint-shell lint-yaml ## Run all linters
123+
@echo "All lint checks passed."
124+
125+
lint-fmt: ## Check Go code formatting (fails if unformatted)
126+
@GOFMT=$$($(BAZEL) run @rules_go//go -- env GOROOT 2>/dev/null | tail -1)/bin/gofmt; \
127+
echo "Checking Go code formatting (using $$GOFMT)..."; \
128+
unformatted=$$($$GOFMT -l .); \
129+
if [ -n "$$unformatted" ]; then \
130+
echo "The following files are not formatted:" >&2; \
131+
echo "$$unformatted" >&2; \
132+
echo "" >&2; \
133+
echo "Please run 'make fmt' and commit the changes." >&2; \
134+
exit 1; \
135+
fi
136+
@echo "All Go files are properly formatted."
137+
138+
lint-json: ## Check JSON files are valid
139+
@echo "Checking JSON files..."
140+
@invalid=0; \
141+
for f in $$(find . -name '*.json' -not -path './bazel-*' -not -path './.git/*' 2>/dev/null); do \
142+
if ! jq . "$$f" > /dev/null 2>&1; then \
143+
echo "Invalid JSON: $$f" >&2; \
144+
invalid=1; \
145+
fi; \
146+
done; \
147+
if [ "$$invalid" -eq 1 ]; then exit 1; fi
148+
@echo "All JSON files are valid."
149+
150+
lint-shell: ## Check shell scripts with shellcheck
151+
@echo "Checking shell scripts..."
152+
@scripts=$$(find . -name '*.sh' -not -path './bazel-*' -not -path './.git/*' 2>/dev/null); \
153+
if [ -n "$$scripts" ]; then \
154+
shellcheck $$scripts; \
155+
fi
156+
@echo "All shell scripts are valid."
157+
158+
lint-yaml: ## Check YAML files with yamllint
159+
@echo "Checking YAML files..."
160+
@yamllint -c .yamllint.yml .
161+
@echo "All YAML files are valid."
162+
86163
local-clean: ## Stop and remove all local services, volumes, and images
87164
@echo "Cleaning all services and data..."
88165
@$(COMPOSE) -f $(COMPOSE_FILE) -p $(LOCAL_PROJECT) down -v --rmi local

core/consumer/registry.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,8 @@ type TopicConfig struct {
5757
// TopicRegistry provides queue, topic name, and subscription config for topics.
5858
// Each topic can have a different queue backend and topic name.
5959
type TopicRegistry struct {
60-
queues map[TopicKey]queue.Queue
61-
topicNames map[TopicKey]string
60+
queues map[TopicKey]queue.Queue
61+
topicNames map[TopicKey]string
6262
subscriptionConfigs map[topicGroup]queue.SubscriptionConfig
6363
}
6464

core/consumer/registry_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ func TestNewTopicRegistry(t *testing.T) {
1818
registry, err := consumer.NewTopicRegistry(
1919
[]consumer.TopicConfig{
2020
{
21-
Key: consumer.TopicKeyRequest,
22-
Name: "request",
21+
Key: consumer.TopicKeyRequest,
22+
Name: "request",
2323
Queue: mockQ,
2424
Subscription: extqueue.DefaultSubscriptionConfig(
2525
"worker-1", "group-a",
@@ -44,7 +44,7 @@ func TestNewTopicRegistry(t *testing.T) {
4444

4545
func TestNewTopicRegistry_InvalidTopicName(t *testing.T) {
4646
tests := []struct {
47-
name string
47+
name string
4848
topicName string
4949
}{
5050
{

entity/build.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ func (s BuildStatus) IsTerminal() bool {
3939
return s == BuildStatusPassed || s == BuildStatusFailed || s == BuildStatusCancelled
4040
}
4141

42-
4342
// SpeculationPathInfo represents the base and head commits of a speculation path used in a build.
4443
type SpeculationPathInfo struct {
4544
// Base is a list of batchIDs(in order) that form the base of this speculation path.

entity/queue/message_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,3 @@ func TestMessage_Fields(t *testing.T) {
6262
assert.Equal(t, msg.PublishedAt, copied.PublishedAt)
6363
assert.Equal(t, msg.Metadata, copied.Metadata)
6464
}
65-

entity/request.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package entity
22

33
import "encoding/json"
44

5-
65
// RequestLandStrategy defines the possible source control integration methods.
76
type RequestLandStrategy string
87

example/server/orchestrator/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ import (
2121
githubchecker "github.com/uber/submitqueue/extension/mergechecker/github"
2222
extqueue "github.com/uber/submitqueue/extension/queue"
2323
queueMySQL "github.com/uber/submitqueue/extension/queue/mysql"
24-
mysqlstorage "github.com/uber/submitqueue/extension/storage/mysql"
2524
"github.com/uber/submitqueue/extension/storage"
25+
mysqlstorage "github.com/uber/submitqueue/extension/storage/mysql"
2626
"github.com/uber/submitqueue/orchestrator/controller"
2727
"github.com/uber/submitqueue/orchestrator/controller/batch"
2828
"github.com/uber/submitqueue/orchestrator/controller/build"
29+
"github.com/uber/submitqueue/orchestrator/controller/buildsignal"
2930
"github.com/uber/submitqueue/orchestrator/controller/conclude"
3031
"github.com/uber/submitqueue/orchestrator/controller/merge"
31-
"github.com/uber/submitqueue/orchestrator/controller/buildsignal"
3232
"github.com/uber/submitqueue/orchestrator/controller/request"
3333
"github.com/uber/submitqueue/orchestrator/controller/score"
3434
"github.com/uber/submitqueue/orchestrator/controller/speculate"

extension/queue/mysql/message_store.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/uber/submitqueue/entity/queue"
1414
)
1515

16-
1716
// sqlmessageStore is the SQL implementation of messageStore
1817
type sqlmessageStore struct {
1918
db *sql.DB
@@ -23,8 +22,8 @@ type sqlmessageStore struct {
2322

2423
// Metric names for message store
2524
const (
26-
metricInsertErrors = "insert.errors"
27-
metricFetchErrors = "fetch.errors"
25+
metricInsertErrors = "insert.errors"
26+
metricFetchErrors = "fetch.errors"
2827
metricMoveToDLQErrors = "move_to_dlq.errors"
2928
)
3029

0 commit comments

Comments
 (0)