Skip to content

Conversation

@daniel-thom
Copy link
Collaborator

No description provided.

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 reorganizes the codebase by restructuring modules and consolidating related functionality. The main changes include moving test utilities to a common test file, creating new module organization for client, server, and configuration components, and migrating to workspace-level dependency management.

Key Changes

  • Migrated torc-slurm-job-runner to use workspace dependencies and updated edition specification
  • Consolidated test utilities into tests/common.rs with comprehensive helper functions for workflow, job, and resource management
  • Reorganized server modules (api, dashboard, routing) into a cleaner structure with proper re-exports

Reviewed changes

Copilot reviewed 2 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
torc-slurm-job-runner/Cargo.toml Migrated to workspace dependencies; updated edition field
tests/common.rs New comprehensive test utilities file with server management, workflow creation, and CLI helpers
src/tui.rs New TUI module with terminal UI implementation and event handling
src/server/dashboard.rs New dashboard module for serving embedded static web assets
src/server/api.rs New common API module with shared context and error handling utilities
src/server.rs New server module organization with proper re-exports
src/lib.rs Updated model re-exports from glob to explicit list
src/config.rs New configuration management module with documentation
src/client/hpc.rs New HPC management module organization
src/client/commands/pagination.rs New pagination utilities module
src/client/commands.rs New commands module with helper functions
src/client/apis.rs New client APIs module with error handling
src/client.rs New client module organization with re-exports

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +40 to +55
// Re-export model types explicitly
pub use models::{
ClaimJobsBasedOnResources, ClaimJobsSortMethod, ClaimNextJobsResponse, ComputeNodeModel,
ComputeNodeSchedule, ComputeNodesResources, CreateJobsResponse, ErrorResponse, EventModel,
FileModel, GetDotGraphResponse, GetReadyJobRequirementsResponse, IsCompleteResponse,
JobDependencyModel, JobFileRelationshipModel, JobModel, JobStatus, JobStatusMap,
JobUserDataRelationshipModel, JobsModel, ListComputeNodesResponse, ListEventsResponse,
ListFilesResponse, ListJobDependenciesResponse, ListJobFileRelationshipsResponse,
ListJobUserDataRelationshipsResponse, ListJobsResponse, ListLocalSchedulersResponse,
ListMissingUserDataResponse, ListRequiredExistingFilesResponse,
ListResourceRequirementsResponse, ListResultsResponse, ListScheduledComputeNodesResponse,
ListSlurmSchedulersResponse, ListUserDataResponse, ListWorkflowsResponse, LocalSchedulerModel,
ProcessChangedJobInputsResponse, ResourceRequirementsModel, ResultModel,
ScheduledComputeNodesModel, SlurmSchedulerModel, UserDataModel, WorkflowActionModel,
WorkflowModel, WorkflowStatusModel,
};
Copy link

Copilot AI Dec 22, 2025

Choose a reason for hiding this comment

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

The comment says "Re-export commonly used types" was changed but it now says "Re-export model types explicitly". However, the list of re-exported types is quite extensive (over 30 types). Consider whether this is truly necessary or if a glob re-export with selective hiding might be more maintainable. If explicit exports are required for documentation or API stability, this is acceptable, but document the reasoning.

Copilot uses AI. Check for mistakes.
@daniel-thom daniel-thom merged commit 8c644b7 into main Dec 22, 2025
2 checks passed
@daniel-thom daniel-thom deleted the refactor/package branch December 22, 2025 18:12
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