Skip to content

Conversation

@rehan892
Copy link
Contributor

🎯 What needed to be done and why?

Ticket: No Ticket

📋 Code Review Cheatsheet

What the reviewer should check
Check Description
🚚 Diff size Is the PR small and focused, or should it be broken into smaller PRs?
🧪 Unit tests Are new features covered with appropriate unit tests?
🥼 Acceptance Criteria met Are the Acceptance Criteria listed in the issue met?
🧐 Code clarity Is the code easy to understand? Are variable and function names meaningful?
🧩 Code organization Are files, modules, and functions structured logically?
🪚 Conciseness Is the code free of unnecessary complexity or redundant code?
💬 Comments & documentation Are code comments explaining why and not how? Is the documentation up-to-date?
🧮 Code consistency Does the code follow existing patterns and conventions in the project?
☣️ Function length Are functions too long? Should they be broken into smaller, more focused functions?
☢️ Class design Are classes well-structured and not overly large or doing too much?
🐾 Logging and debugging statements Are print/debug statements removed or replaced with proper logging?

@rehan892 rehan892 requested review from calvinhp and daveoconnor May 20, 2025 15:42
@rehan892 rehan892 changed the title Update makefile targets and update dockerfile fix: Add cleanup makefile target / project specific container names May 20, 2025
@daveoconnor
Copy link

Is it a requirement that a person be able to run multiple projects side by side, say they're switching between projects frequently within a day, or conference demos?

Copy link

@daveoconnor daveoconnor left a comment

Choose a reason for hiding this comment

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

Approving.

After discussing with Rehan we're going with another ticket to address questions around multiple projects

#8

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.

3 participants