Skip to content

Conversation

@zhenghaoz
Copy link
Collaborator

No description provided.

@zhenghaoz zhenghaoz requested a review from Copilot October 12, 2025 04:13
Copy link
Contributor

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 removes the base package by moving its functionality to more appropriate packages within the codebase. The refactoring consolidates utility functions, data structures, and helper methods into common/util, common/jsonutil, common/copier, and dataset packages for better organization.

  • Moves core utility functions (random generators, panic handling, matrix operations) from base to common/util
  • Relocates indexing and data structure functionality from base to dataset package
  • Updates import statements across all affected files to reference the new package locations

Reviewed Changes

Copilot reviewed 30 out of 31 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
worker/worker_test.go Updates imports and function calls to use dataset package instead of base
worker/worker.go Replaces base imports with util package for random generation and panic handling
storage/data/sql.go Moves jsonutil import from base/jsonutil to common/jsonutil
storage/data/database.go Updates jsonutil import path to use common package
server/server.go Replaces base.CheckPanic() with util.CheckPanic()
model/model.go Updates random generator imports and types to use util package
model/ctr/optimize_test.go Changes index creation to use dataset package functions
model/ctr/model.go Updates unified index types and removes Clone function, updates imports
model/ctr/data_test.go Renames variables and updates package references from base to dataset
model/ctr/data.go Updates imports and index types to use dataset and util packages
model/cf/model.go Removes Clone function and updates imports to use util for matrix operations
master/tasks.go Updates index creation and validation functions to use dataset package
master/rpc_test.go Updates test dataset creation to use dataset package
master/rest.go Replaces base.ValidateId calls with util.ValidateId
master/master.go Updates panic handling to use util.CheckPanic()
dataset/ files Updates package declarations from base to dataset
common/util/ files Updates package declarations from base to util
common/jsonutil/json_test.go Minor import ordering fix
common/copier/ files Minor import ordering fixes
common/parallel/ files Updates imports and panic handling to use util package
Comments suppressed due to low confidence (1)

model/ctr/model.go:162

  • The Spawn function should handle the case where the original Clone function was removed. Consider adding a comment explaining why this simplified implementation is sufficient, or ensure that all callers can handle this change appropriately.
func Spawn(m FactorizationMachines) FactorizationMachines {
	if cloner, ok := m.(FactorizationMachineSpawner); ok {
		return cloner.Spawn()
	}
	return m
}

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@zhenghaoz zhenghaoz merged commit 2b28f85 into master Oct 12, 2025
11 checks passed
@zhenghaoz zhenghaoz deleted the remove_base branch October 12, 2025 04:22
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