Skip to content

Conversation

@julio-lopez
Copy link
Collaborator

@julio-lopez julio-lopez commented Jul 1, 2025

  • use slices.Clone
  • remove stale .gometalinter.json
  • unexport maintenance.dropDeletedContents
  • rename fetchIndexBlob
  • use require in TestTimeFuncWiring

@julio-lopez julio-lopez requested a review from Copilot July 1, 2025 18:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR includes cleanup and refactoring across multiple packages, improving naming consistency and test assertions while removing obsolete lint configurations.

  • Use slices.Clone in SortByTime for clearer slice copying.
  • Rename the exported function DropDeletedContents to unexported dropDeletedContents and update references.
  • Refactor tests to use require assertions and remove an outdated .gometalinter.json.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
snapshot/manifest.go Switched to slices.Clone instead of manual slice append.
repo/maintenance/maintenance_run.go Updated call from DropDeletedContents to dropDeletedContents.
repo/maintenance/drop_deleted_contents.go Renamed DropDeletedContents to dropDeletedContents.
repo/content/committed_content_index.go Renamed fetchOne to fetchIndexBlob for clarity.
internal/repotesting/repotesting_test.go Replaced manual error checks with require assertions.
cli/command_snapshot_verify.go Removed //nolint:wrapcheck directive.
.gometalinter.json Deleted obsolete linter configuration.
Comments suppressed due to low confidence (4)

repo/maintenance/drop_deleted_contents.go:12

  • Making DropDeletedContents unexported is a breaking change in the repo API; consider providing a deprecated alias or ensuring no external callers rely on this function.
func dropDeletedContents(ctx context.Context, rep repo.DirectRepositoryWriter, dropDeletedBefore time.Time, safety SafetyParameters) error {

internal/repotesting/repotesting_test.go:77

  • [nitpick] The error message 'failed to get create snapshot' is grammatically incorrect; consider using 'failed to create snapshot' for clarity.
	require.NoError(t, err, "failed to get create snapshot")

@julio-lopez julio-lopez changed the title Draft/nits chore(general): minor cleanups Jul 1, 2025
@codecov
Copy link

codecov bot commented Jul 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.38%. Comparing base (cb455c6) to head (6fa7df3).
Report is 592 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4704      +/-   ##
==========================================
+ Coverage   75.86%   76.38%   +0.51%     
==========================================
  Files         470      530      +60     
  Lines       37301    40371    +3070     
==========================================
+ Hits        28299    30838    +2539     
- Misses       7071     7494     +423     
- Partials     1931     2039     +108     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@julio-lopez julio-lopez requested a review from Rohit-BM18 July 1, 2025 18:58
@julio-lopez julio-lopez marked this pull request as ready for review July 1, 2025 18:59
@julio-lopez
Copy link
Collaborator Author

cc: @andrason

@julio-lopez julio-lopez merged commit 89d1bbc into kopia:master Jul 1, 2025
27 checks passed
@julio-lopez julio-lopez deleted the draft/nits branch July 1, 2025 19:05
Copy link
Collaborator

@Rohit-BM18 Rohit-BM18 left a comment

Choose a reason for hiding this comment

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

LGTM

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