Skip to content

Sprint 2 Finished!#4

Open
Jameson789 wants to merge 59 commits intokellerflint:mainfrom
Jameson789:main
Open

Sprint 2 Finished!#4
Jameson789 wants to merge 59 commits intokellerflint:mainfrom
Jameson789:main

Conversation

@Jameson789
Copy link
Copy Markdown

No description provided.

instructions.js Outdated
return COLOR_ABBR[id] || (typeof id === 'string' ? id.slice(0, 2).toUpperCase() : '??');
}

function rle(arr) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What does this function do? This function name does not seem to explain well what it is accomplishing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This server entry point file is done very well. Organized well and is very easy to read. good job!

instructions.js Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This file was quite difficult to understand. Some clarifying comments would have been nice.

instructions.js Outdated
const m = /^rgb\((\d+),\s*(\d+),\s*(\d+)\)$/.exec(rgb);
if (!m) return rgb; // already hex or unknown format
const h = n => Number(n).toString(16).padStart(2, '0');
return `#${h(m[1])}${h(m[2])}${h(m[3])}`;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What does this do? Lack of comments, very confusing to understand

queueLimit: 0,
});

export default pool;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Love the simplicity of the db

Copy link
Copy Markdown
Contributor

@AlexanderORuban AlexanderORuban left a comment

Choose a reason for hiding this comment

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

  • Break up app.js to use routers and controllers for better organization
  • instructions.js is confusing due to a lack of comments
  • Variable names could be longer to give more context

kellerflint pushed a commit that referenced this pull request Oct 16, 2025
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.

5 participants