Open
Conversation
…updated, and file.infected
Previously, any status not explicitly handled (e.g. 409, 403, 500, 502)
fell through to the JSON decode path, silently producing zero-value
results. Now all >= 400 statuses return an error.
Also fix Decode(&resdata) -> Decode(resdata) to avoid unnecessary
*interface{} indirection.
Extract handleResponse method with defer resp.Body.Close() to guarantee the response body is closed on all paths including nil resdata, error decoding, and retry. Replace goto with for loop.
Derive CDN base URL from the public key (SHA256 → base36 → first 10 chars). Config.CDNBase can be set explicitly to override the default. Shallow-copy Config in resolveConfig to avoid mutating a shared template when creating clients for multiple projects.
New projectapi/ package covering projects CRUD, secret keys management, and usage metrics. Adds ucare.NewBearerClient() for bearer token auth used by the Project API.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds new packages (addon, metadata, projectapi), expands public APIs (file.Info accepts params; webhook.Delete by ID), introduces bearer-token Project API client with opt-in retry, exported structured API errors, CDN helpers, conversion path builders, unified Upload with metadata support, and various tests and docs updates. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
conversion/path.go (2)
56-58: Consider validating requiredFormatfield.Unlike
BuildDocumentPathwhich defaultsFormatto"pdf",BuildVideoPathuses the format directly. An emptyFormatwould produce a malformed path like"uuid/video/-/format//".📝 Option: Add validation for empty Format
func BuildVideoPath(opts VideoPathOptions) string { + if opts.Format == "" { + // Consider returning an error or setting a default + } var b strings.Builder fmt.Fprintf(&b, "%s/video/-/format/%s/", opts.UUID, opts.Format)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conversion/path.go` around lines 56 - 58, BuildVideoPath currently uses opts.Format directly and can produce a malformed path when Format is empty; update the function that consumes VideoPathOptions (BuildVideoPath) to mirror BuildDocumentPath behavior by defaulting opts.Format to a sensible value (e.g., "mp4") when it is empty or validate and return an error upstream; specifically, inside BuildVideoPath check if opts.Format == "" and set opts.Format = "mp4" (or alternatively perform validation and fail fast), then continue building the path so the "%s/video/-/format/%s/" segment never contains an empty format string.
71-73: Silent failure when only one cut parameter is provided.If a caller provides only
CutStartor onlyCutLength, both are silently ignored. This could lead to unexpected behavior where the user expects a cut operation but gets the full video.Consider either documenting this behavior clearly, or returning an error when the parameters are partially specified.
📝 Option: Add validation or document the behavior
If validation is preferred:
+// ErrPartialCutParams is returned when only one of CutStart or CutLength is provided. +var ErrPartialCutParams = errors.New("both CutStart and CutLength must be provided for cut operation") + // BuildVideoPath constructs a video conversion path. -func BuildVideoPath(opts VideoPathOptions) string { +func BuildVideoPath(opts VideoPathOptions) (string, error) { + if (opts.CutStart != "" && opts.CutLength == "") || (opts.CutStart == "" && opts.CutLength != "") { + return "", ErrPartialCutParams + }Alternatively, add a doc comment on
VideoPathOptionsexplaining that bothCutStartandCutLengthmust be set together.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@conversion/path.go` around lines 71 - 73, The code that builds the cut segment ignores CutStart/CutLength when only one is set; add validation in the path-construction function that consumes VideoPathOptions (the function that currently checks opts.CutStart and opts.CutLength) to detect partial specification: if one of opts.CutStart or opts.CutLength is non-empty while the other is empty, return an error (or a well-defined sentinel) instead of silently omitting the cut, and update callers to handle that error; alternatively (if you prefer non-failing behavior) add a doc comment to the VideoPathOptions type clearly stating that CutStart and CutLength must be provided together.README.md (1)
180-181: Minor: Inconsistent line break formatting.Line 180 ends without the double-space line break (
) that other links in this section use. This may cause the "Changelog" link to render on the same line as "REST API reference" in some Markdown renderers.📝 Fix formatting
-[REST API reference](https://uploadcare.com/api-refs/rest-api/?utm_source=github&utm_medium=referral&utm_campaign=uploadcare-go) +[REST API reference](https://uploadcare.com/api-refs/rest-api/?utm_source=github&utm_medium=referral&utm_campaign=uploadcare-go)(add two trailing spaces)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 180 - 181, The markdown links "REST API reference" and "Changelog" are missing a trailing double-space line break between them; update the README.md so the line containing the "REST API reference" link ends with two spaces (add " " after the URL) so "Changelog" renders on the next line; locate the lines containing the "REST API reference" and "Changelog" link texts to apply the fix.ucare/cdn.go (1)
16-22: Consider checkingSetStringreturn value for defensive coding.While
hexStrfromhex.EncodeToStringis always valid hex, ignoring the boolean return fromSetStringcould mask bugs if this code is refactored later.🛡️ Proposed defensive fix
func CnamePrefix(pubkey string) string { hash := sha256.Sum256([]byte(pubkey)) hexStr := hex.EncodeToString(hash[:]) n := new(big.Int) - n.SetString(hexStr, 16) + if _, ok := n.SetString(hexStr, 16); !ok { + panic("invalid hex from sha256 hash") + } return n.Text(36)[:10] }Note: Uploadcare's actual CDN subdomain generation algorithm is not publicly documented, so this implementation cannot be verified against their official specification. The algorithm appears to be a custom implementation specific to this codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ucare/cdn.go` around lines 16 - 22, The CnamePrefix function currently ignores the boolean return of n.SetString so add defensive handling: capture the bool result from n.SetString(hexStr, 16) and if it returns false, handle the error path (e.g., return a safe fallback such as the first 10 chars of hexStr or log/return an error) instead of proceeding; update the code around CnamePrefix and the n.SetString call to check the return value and return the chosen fallback when parsing fails.test/integration_test.go (1)
110-113: Include root-cause details when project discovery fails.At Line 112, the fatal message collapses two distinct failures (request error vs empty results), which makes debugging CI failures harder.
🔧 Suggested improvement
list, err := svc.List(context.Background(), nil) - if err != nil || len(list.Results) == 0 { - t.Fatal("cannot list projects to get pub_key for further tests") - } + if err != nil { + t.Fatalf("cannot list projects to get pub_key for further tests: %v", err) + } + if len(list.Results) == 0 { + t.Fatal("cannot list projects to get pub_key for further tests: no projects returned") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration_test.go` around lines 110 - 113, The current test collapses request errors and empty responses into one generic t.Fatal; update the svc.List call handling so you first check and fail on a non-nil err (use t.Fatalf and include the err value and any response metadata), and only after that assert len(list.Results) > 0 and fail with a clear message that includes the actual list or its length; target the svc.List call and the t.Fatal/t.Fatalf invocation so the failure distinguishes request error vs empty results.projectapi/usage.go (1)
30-46: Consider validating themetricparameter.The
metricparameter is interpolated directly into the URL path. While the comment documents valid values ("traffic", "storage", "operations"), there's no validation to prevent misuse. Invalid metrics would be rejected server-side, but client-side validation provides faster feedback.Optional: Add metric validation
// Metric constants for GetUsageMetric const ( MetricTraffic = "traffic" MetricStorage = "storage" MetricOperations = "operations" )Then optionally validate in
GetUsageMetricor document that validation is deferred to the server.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projectapi/usage.go` around lines 30 - 46, GetUsageMetric interpolates the metric into the path without validating it; add constants (MetricTraffic, MetricStorage, MetricOperations) and validate the metric at the start of GetUsageMetric (or expose a helper isValidMetric) and return an error (e.g., fmt.Errorf("invalid metric: %q", metric)) if it is not one of the allowed values before calling s.svc.ResourceOp to avoid constructing invalid URLs.test/addon.go (1)
18-18: Useassert.NoErrorfor error assertions.Using
assert.Equal(t, nil, err)works but is not idiomatic.assert.NoError(t, err)provides clearer error messages when assertions fail and is the recommended pattern in testify.Suggested fix
- assert.Equal(t, nil, err) + assert.NoError(t, err)Apply similarly at line 27.
Also applies to: 27-27
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/addon.go` at line 18, Replace the non-idiomatic error assertions using assert.Equal(t, nil, err) with assert.NoError(t, err) in the test file so failures report clearer error messages; update the two occurrences (the one at line with the original diff and the similar occurrence around line 27) to use assert.NoError(t, err).test/metadata.go (1)
14-14: Preferassert.NoErroroverassert.Equal(t, nil, err).The
assert.NoError(t, err)idiom provides better failure messages and is more idiomatic with testify. It's also consistent withtest/projectapi.gowhich usesassert.NoError.♻️ Suggested refactor
func metadataSet(t *testing.T, r *testenv.Runner) { ctx := context.Background() val, err := r.Metadata.Set(ctx, r.Artifacts.Files[0].ID, "test_key", "test_value") - assert.Equal(t, nil, err) + assert.NoError(t, err) assert.Equal(t, "test_value", val) } func metadataGet(t *testing.T, r *testenv.Runner) { ctx := context.Background() val, err := r.Metadata.Get(ctx, r.Artifacts.Files[0].ID, "test_key") - assert.Equal(t, nil, err) + assert.NoError(t, err) assert.Equal(t, "test_value", val) } func metadataList(t *testing.T, r *testenv.Runner) { ctx := context.Background() data, err := r.Metadata.List(ctx, r.Artifacts.Files[0].ID) - assert.Equal(t, nil, err) + assert.NoError(t, err) assert.Equal(t, "test_value", data["test_key"]) } func metadataDelete(t *testing.T, r *testenv.Runner) { ctx := context.Background() err := r.Metadata.Delete(ctx, r.Artifacts.Files[0].ID, "test_key") - assert.Equal(t, nil, err) + assert.NoError(t, err)Also applies to: 21-21, 28-28, 35-35
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/metadata.go` at line 14, Replace occurrences of the non-idiomatic assertion assert.Equal(t, nil, err) with assert.NoError(t, err); locate all uses of the exact call assert.Equal(t, nil, err) (including the ones referenced at lines showing assert.Equal) and change them to assert.NoError to get better failure messages and match the existing test style (e.g., as used in test/projectapi.go).ucare/restapi.go (1)
58-63: Retry replay shouldn't mutate the live request.
req.GetBody()currently re-runsdata.EncodeReq(req)against the same*http.Request. Any encoder that appends query params or headers will accumulate state on attempt 2+, so the retried request may not match the original one byte-for-byte. Prefer replaying from a cloned request or a snapshotted encoded payload.Also applies to: 80-85, 101-108
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ucare/restapi.go` around lines 58 - 63, getBodyBuilder currently calls data.EncodeReq(req) on each replay which mutates the original *http.Request; change it so EncodeReq is only run once and replay uses an immutable snapshot: call data.EncodeReq(req) a single time into a buffer (or clone the request first), capture the encoded payload and any headers/query state, then return a closure that produces a fresh io.ReadCloser each attempt (e.g., a new bytes.Reader/io.NopCloser over a copy of the buffer) and reapply any non-idempotent headers/query additions from the snapshot rather than re-running EncodeReq; apply the same pattern to the other request-replay helpers referenced (the functions around the other diff regions).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@file/service.go`:
- Line 21: The file.Service interface change is breaking; restore the original
method signature Info(ctx context.Context, id string) (Info, error) and add a
new non-breaking method such as InfoWithParams(ctx context.Context, id string,
params *InfoParams) (Info, error) (or InfoWithOptions) to support the optional
params. Update the file.Service interface to include both Info and
InfoWithParams, then update any concrete implementations (types that implement
file.Service) to implement both methods (have Info call InfoWithParams(ctx, id,
nil) or vice versa), and update callers to use the new InfoWithParams when they
need params while leaving existing calls to Info unchanged.
In `@projectapi/projects.go`:
- Around line 53-67: The Update method on service (func (s service) Update(...))
calls a non-existent Uploadcare Project API endpoint (using projectPathFmt and
UpdateProjectParams); remove this method from the service API or refactor it to
return an explicit unsupported-operation error (e.g., ErrUnsupportedOperation)
and update any callers to handle that error; ensure references to projectPathFmt
and UpdateProjectParams are cleaned up if you delete the method, or replace the
implementation with a clear error-returning stub and documentation comment
stating project updates must be done via the Uploadcare Dashboard.
In `@ucare/restapi.go`:
- Around line 101-110: The debug log currently uses log.Debugf("making %d
request: %+v", tries, req) which prints the full http.Request (including Headers
and Authorization) and can leak credentials; change this to log only
non-sensitive request attributes (e.g., method, URL, ContentLength, and
optionally Host) and do NOT include req.Header or %+v; update the retry loop
around req/GetBody/Body (the variables in this block) to use the new safer log
call so retries no longer emit headers or token data.
---
Nitpick comments:
In `@conversion/path.go`:
- Around line 56-58: BuildVideoPath currently uses opts.Format directly and can
produce a malformed path when Format is empty; update the function that consumes
VideoPathOptions (BuildVideoPath) to mirror BuildDocumentPath behavior by
defaulting opts.Format to a sensible value (e.g., "mp4") when it is empty or
validate and return an error upstream; specifically, inside BuildVideoPath check
if opts.Format == "" and set opts.Format = "mp4" (or alternatively perform
validation and fail fast), then continue building the path so the
"%s/video/-/format/%s/" segment never contains an empty format string.
- Around line 71-73: The code that builds the cut segment ignores
CutStart/CutLength when only one is set; add validation in the path-construction
function that consumes VideoPathOptions (the function that currently checks
opts.CutStart and opts.CutLength) to detect partial specification: if one of
opts.CutStart or opts.CutLength is non-empty while the other is empty, return an
error (or a well-defined sentinel) instead of silently omitting the cut, and
update callers to handle that error; alternatively (if you prefer non-failing
behavior) add a doc comment to the VideoPathOptions type clearly stating that
CutStart and CutLength must be provided together.
In `@projectapi/usage.go`:
- Around line 30-46: GetUsageMetric interpolates the metric into the path
without validating it; add constants (MetricTraffic, MetricStorage,
MetricOperations) and validate the metric at the start of GetUsageMetric (or
expose a helper isValidMetric) and return an error (e.g., fmt.Errorf("invalid
metric: %q", metric)) if it is not one of the allowed values before calling
s.svc.ResourceOp to avoid constructing invalid URLs.
In `@README.md`:
- Around line 180-181: The markdown links "REST API reference" and "Changelog"
are missing a trailing double-space line break between them; update the
README.md so the line containing the "REST API reference" link ends with two
spaces (add " " after the URL) so "Changelog" renders on the next line; locate
the lines containing the "REST API reference" and "Changelog" link texts to
apply the fix.
In `@test/addon.go`:
- Line 18: Replace the non-idiomatic error assertions using assert.Equal(t, nil,
err) with assert.NoError(t, err) in the test file so failures report clearer
error messages; update the two occurrences (the one at line with the original
diff and the similar occurrence around line 27) to use assert.NoError(t, err).
In `@test/integration_test.go`:
- Around line 110-113: The current test collapses request errors and empty
responses into one generic t.Fatal; update the svc.List call handling so you
first check and fail on a non-nil err (use t.Fatalf and include the err value
and any response metadata), and only after that assert len(list.Results) > 0 and
fail with a clear message that includes the actual list or its length; target
the svc.List call and the t.Fatal/t.Fatalf invocation so the failure
distinguishes request error vs empty results.
In `@test/metadata.go`:
- Line 14: Replace occurrences of the non-idiomatic assertion assert.Equal(t,
nil, err) with assert.NoError(t, err); locate all uses of the exact call
assert.Equal(t, nil, err) (including the ones referenced at lines showing
assert.Equal) and change them to assert.NoError to get better failure messages
and match the existing test style (e.g., as used in test/projectapi.go).
In `@ucare/cdn.go`:
- Around line 16-22: The CnamePrefix function currently ignores the boolean
return of n.SetString so add defensive handling: capture the bool result from
n.SetString(hexStr, 16) and if it returns false, handle the error path (e.g.,
return a safe fallback such as the first 10 chars of hexStr or log/return an
error) instead of proceeding; update the code around CnamePrefix and the
n.SetString call to check the return value and return the chosen fallback when
parsing fails.
In `@ucare/restapi.go`:
- Around line 58-63: getBodyBuilder currently calls data.EncodeReq(req) on each
replay which mutates the original *http.Request; change it so EncodeReq is only
run once and replay uses an immutable snapshot: call data.EncodeReq(req) a
single time into a buffer (or clone the request first), capture the encoded
payload and any headers/query state, then return a closure that produces a fresh
io.ReadCloser each attempt (e.g., a new bytes.Reader/io.NopCloser over a copy of
the buffer) and reapply any non-idempotent headers/query additions from the
snapshot rather than re-running EncodeReq; apply the same pattern to the other
request-replay helpers referenced (the functions around the other diff regions).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2f9f5796-ffc0-47c3-8818-40c7cacaff53
📒 Files selected for processing (65)
CHANGELOG.mdREADME.mdaddon/addon.goaddon/log.goaddon/service.goaddon/service_test.goconversion/conversion.goconversion/conversion_test.goconversion/path.goconversion/path_test.gofile/file.gofile/list.gofile/service.gofile/service_test.gogroup/group.gogroup/group_test.gogroup/service.gointernal/codec/codec.gointernal/codec/codec_test.gointernal/config/config.gometadata/log.gometadata/metadata.gometadata/service.gometadata/service_test.gometadata/validation.goprojectapi/log.goprojectapi/params.goprojectapi/projectapi.goprojectapi/projects.goprojectapi/secrets.goprojectapi/service.goprojectapi/service_test.goprojectapi/usage.gotest/addon.gotest/file.gotest/group.gotest/integration_test.gotest/metadata.gotest/projectapi.gotest/testenv/runner.gotest/webhook.goucare/cdn.goucare/cdn_test.goucare/client.goucare/doc.goucare/error.goucare/error_test.goucare/projectapi.goucare/projectapi_test.goucare/restapi.goucare/restapi_test.goucare/retry.goucare/retry_test.goucare/uploadapi.goucare/uploadapi_test.goupload/direct.goupload/fromurl.goupload/multipart.goupload/service.goupload/upload.goupload/upload_test.gowebhook/service.gowebhook/service_test.gowebhook/webhook.gowebhook/webhook_test.go
💤 Files with no reviewable changes (1)
- internal/config/config.go
Pagination next URLs point to a different host (app.uploadcare.com), so List and ListSecrets now use codec.ResultBuf iterators that automatically follow next links. Bearer client fallback routes unknown hosts through the same auth backend.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
projectapi/projects.go (1)
53-67:⚠️ Potential issue | 🔴 Critical
Updateappears to target a non-existent Project API operation.Please verify that
POST /projects/{pubKey}/is actually supported. If not, this method should be removed or replaced with an explicit unsupported-operation error, and references in the service interface/docs should be aligned.Does Uploadcare Project API currently support updating project settings via API? If yes, provide the exact HTTP method and endpoint path for updating a project by public key.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@projectapi/projects.go` around lines 53 - 67, The Update method appears to call a non-existent API operation (using http.MethodPost to fmt.Sprintf(projectPathFmt, pubKey)); verify whether the Uploadcare Project API supports updating project settings and if so confirm the exact HTTP method and endpoint and update projectPathFmt/HTTP verb accordingly; if the API does not support this operation remove the Update method (and any UpdateProjectParams type and references) or change it to return a clear unsupported-operation error, and update the service interface/docs to match; check symbols: Update, UpdateProjectParams, projectPathFmt, and svc.ResourceOp to locate and fix or remove the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ucare/client.go`:
- Around line 125-134: NewRequest currently returns errNoClient when endpoint
(u.Host) isn't in client.backends, so fallbackDo is never used for cross-host
pagination; update NewRequest to detect a missing backend and, if
client.fallbackDo is set, use the fallback client to build and execute the
request instead of returning errNoClient (e.g., delegate creation/execution to
the same client used by fallbackDo), and ensure NewBearerClient assigns
fallbackDo on the constructed client so cross-host next/previous URLs are routed
through the bearer-auth fallback path; key symbols: client struct, NewRequest,
fallbackDo, NewBearerClient, errNoClient.
---
Duplicate comments:
In `@projectapi/projects.go`:
- Around line 53-67: The Update method appears to call a non-existent API
operation (using http.MethodPost to fmt.Sprintf(projectPathFmt, pubKey)); verify
whether the Uploadcare Project API supports updating project settings and if so
confirm the exact HTTP method and endpoint and update projectPathFmt/HTTP verb
accordingly; if the API does not support this operation remove the Update method
(and any UpdateProjectParams type and references) or change it to return a clear
unsupported-operation error, and update the service interface/docs to match;
check symbols: Update, UpdateProjectParams, projectPathFmt, and svc.ResourceOp
to locate and fix or remove the implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54f7a96a-f0d3-4a57-a410-aab15fde2ba3
📒 Files selected for processing (8)
projectapi/projectapi.goprojectapi/projects.goprojectapi/secrets.goprojectapi/service.goprojectapi/service_test.gotest/integration_test.gotest/projectapi.goucare/client.go
✅ Files skipped from review due to trivial changes (2)
- projectapi/service_test.go
- test/projectapi.go
🚧 Files skipped from review as they are similar to previous changes (1)
- projectapi/secrets.go
Pagination next URLs point to a different host (app.uploadcare.com). NewRequest was failing with errNoClient before Do/fallbackDo could run. Add fallbackNewReq to route unknown endpoints through the bearer auth client for both request construction and execution.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ucare/projectapi_test.go (1)
12-14: Consider using standard import alias forrequirepackage.The alias
assertforgithub.com/stretchr/testify/requireis technically valid but may cause confusion since testify also has anassertpackage with different semantics (requirestops on first failure,assertcontinues). The common convention is to use the package name directly or alias it asrequire.♻️ Suggested change
- assert "github.com/stretchr/testify/require" + "github.com/stretchr/testify/require"Then update all
assert.calls torequire.throughout the file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ucare/projectapi_test.go` around lines 12 - 14, Replace the import alias `assert` for "github.com/stretchr/testify/require" with the standard `require` name and update all call sites in this test file (e.g., any `assert.` usages) to `require.` so the semantics match the package (functions like require.NoError, require.Equal) and avoid confusion with the separate testify `assert` package.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ucare/projectapi_test.go`:
- Around line 12-14: Replace the import alias `assert` for
"github.com/stretchr/testify/require" with the standard `require` name and
update all call sites in this test file (e.g., any `assert.` usages) to
`require.` so the semantics match the package (functions like require.NoError,
require.Equal) and avoid confusion with the separate testify `assert` package.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ed30fa52-6bca-4a82-bcc4-a9335e93fd1a
📒 Files selected for processing (2)
ucare/client.goucare/projectapi_test.go
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 by CodeRabbit
New Features
API Updates
Error Handling