refactor: align codebase with STANDARDS.md#160
Merged
Conversation
Replace all testify/assert and testify/require usage across 69 test files with shared/testutil assertion helpers. This aligns with STANDARDS.md Section 14: no testify, no gomega, no ginkgo. - Add shared/testutil/assert.go with Equal, NoError, Contains, True, False, Nil, NotNil, Len, Empty, and Require* variants - Convert all test files in shared/, tools/cfl/, and tools/jtk/ - Remove testify dependency from all three go.mod files - Add STANDARDS.md to repo root - Update docs to reference shared/testutil Refs #159
Remove hardcoded context.Background() convenience wrappers from the jtk API client and add context.Context as the first parameter to all 58+ exported API methods, matching the pattern already used by the cfl tool. Update all command callers to pass cmd.Context() and all tests to pass context.Background().
Replace "failed to X:" error context with gerund noun phrases ("Xing:")
across all 56 source files and 16 test files. Convert sentinel errors
in jtk/api/client.go from fmt.Errorf to errors.New.
Add revive, gosec, errorlint, and exhaustive to golangci-lint configs for all three modules (shared, cfl, jtk) per STANDARDS.md Section 1. Fix all violations: - errorlint: use errors.Is() instead of == for error comparisons - exhaustive: add missing cases to switch statements - gosec: handle errors from w.Write/json.Encode in test handlers, use restrictive file permissions (0600/0750), add nolint for intentional os.Open/exec.Command usage - revive: rename unused parameters to _, add package doc comments, fix exported type/const documentation, add nolint for intentional api package name Refs #159
Add comprehensive test coverage for all previously untested jtk command packages: - root: command structure, flags, defaults, View(), SetAPIClient() - boards: list (table/JSON/empty), get (table/JSON/invalid ID) - completion: bash/zsh/fish/powershell shell completion output - me: table/JSON/plain output, email handling, auth failure - users: search (table/JSON/empty, active/inactive filtering) - sprints: list/current/issues/add with table/JSON/empty/error cases - attachments: list/add/get/delete with file I/O and API mocking 58 new tests total, all using httptest and shared/testutil assertions. Refs #159
Add `make check` (tidy, lint, test, build) as the CI gate target per STANDARDS.md. Add `make tidy` with dirty-check to all three Makefiles. Root `make build` now outputs to bin/. CI jobs verify modules are tidy before building.
- Fix jtk client.go returning ErrNotFound instead of ErrURLRequired for empty URL; add errors.Is assertions to client tests - Wire context.Context through jtk AddAttachment and DownloadAttachment (previously discarded ctx and used http.NewRequest) - Add context.Context parameter to GetCloudID and AutomationBaseURL; remove context.Background() from internal GetCloudID call - Wire context through cfl verifyConnection using http.NewRequestWithContext
Add signal.NotifyContext with SIGINT/SIGTERM to both main.go entry points so Ctrl+C cancels in-flight operations cleanly. Use ExecuteContext(ctx) to propagate the signal-aware context through the Cobra command chain. Update all cfl run* functions and their tests to accept and pass context.Context as the first parameter.
…pping
Add six new sentinel errors to jtk api/errors.go for validation:
ErrAttachmentIDRequired, ErrFilePathRequired, ErrAttachmentRequired,
ErrAttachmentContentMissing, ErrCommentIDRequired, ErrTaskIDRequired.
Replace all fmt.Errorf("X is required") with sentinel errors across
both tools. Wrap ~60 bare `return err` statements with noun-phrase
context (e.g., "fetching issue: %w"). Fix verb-phrase wrapping like
"request failed: %w" to noun-phrase style "uploading attachment: %w".
… pre-allocate slices
- Replace interface{} with any across all Go files in both tools
- Replace deprecated strings.Title with cases.Title from golang.org/x/text
- Pre-allocate row slices in list/search commands where collection size is known
- Use nil slice instead of empty literal in configcmd/clear.go
- Fix gofmt formatting issues from prior phases
Move confirmation prompts, status messages, and verbose logging from stdout to stderr so stdout remains clean for pipeable structured data.
…e fix - Wrap 21 bare error returns with context (19 in jtk/api, 2 in cfl/api) - Fix 3 "failed to" prefixes to verb-phrase error wrapping - Remove log.Printf from parser.go (warnings stored in struct) - Document intentional error discard in shared/errors - Add t.Helper() to 5 cfl test helpers - Add t.Parallel() to ~900 test functions across all 3 modules - Fix data race: remove global color.NoColor mutation in view.New() - Fix cfl/go.mod: golang.org/x/text indirect→direct
- Replace interface{} with any across shared packages (client, view,
testutil, adf)
- Add ctx.Err() checks to pagination loops in SearchAll and
ListAutomationRules for cancellation responsiveness
- Add t.Parallel() to test functions without shared mutable state
- Fix race condition in automation_test.go pagination counter using
sync/atomic
- Add package doc comment to jtk api package
- Wrap bare error return in APIError.UnmarshalJSON
…DS.md Bring the dashboards (#165), links (#164), and text packages — added to main after the branch diverged — into alignment with the project coding standards applied by earlier commits in this branch: - Replace c.get/post/delete helpers with context-aware c.Get/Post/Delete - Convert testify assertions to shared/testutil helpers - Add _ = to unhandled json.NewEncoder errors (gosec G104) - Rename unused cmd/r parameters to _ (revive unused-parameter) - Add package doc comments (revive package-comments) - Wire context.Context through changeIssueType
rianjs
force-pushed
the
refactor/standards-alignment
branch
from
d1890a0 to
56d3e35
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive refactoring to align the codebase with STANDARDS.md conventions. Covers all three modules (shared, cfl, jtk) across 198 files.
shared/testutilpackage (Section 14)context.Contextas first parameter to all 58+ jtk API methods (Section 5)Closes #159
Test plan
make allpasses (build + test + lint) — 41 packages, 0 failures, 0 lint issuesgrep -r "stretchr/testify" tools/ shared/ --include="*.go"returns 0 matches