Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code refactor #190

Merged
merged 4 commits into from
Oct 16, 2023
Merged

Code refactor #190

merged 4 commits into from
Oct 16, 2023

Conversation

maciejt10c
Copy link
Contributor

10minions-vscode review

  • README.md:
    • In "Limitations" I'd add that the extension working speed is mainly limited by the speed of OpenAI GPT-4 API.
  • package.json:
    • Missing license field. It's also a good idea to add LICENSE.md.
    • It seems there are unused dependencies, like firebase.
    • npm audit reports 3 vulnerabilities (2 moderate, 1 critical). Removing unused deps may fix this, at least partially.
  • SideBarWebViewInnerComponent.tsx:
  • MinionTaskComponent.tsx:
    • insanely long components, can probably be split into smaller ones
  • MinionIconsOutline.tsx
  • MinionIconsFill.tsx
    • is there no way to keep SVGs as separate files instead of inlining them in tsx? This looks ugly and is hard to maintain.

      VSMinionTaskAutoRunner.ts:
  • I'd change if (line.includes('//TODO:')) { to if (/\/\/ ?TODO:/.test(line)) {, or something even more generic (as it won't work for languages that have other styles of comments, like python). All TODO comments in my projects use space before TODO.
  • .filter((file) => path.extname(file) === '.ts'); Why there's no .js, '.jsxor.tsx` support?

    extractExecutionIdFromUri.ts:
  • /^minionTaskId\/([a-z\d\-]+)\/.*/ should be changed to /^minionTaskId\/([a-z\d\-]+)\//, as the last part is not used.
  • VSMinionTasksManager.ts:
    • in acquireMinionIndex: Shouldn't NUM_TOTAL_ROBOTS be equal to ALL_FILL_ROBOT_ICONS.length instead of being hardcoded?

@maciejt10c maciejt10c merged commit c7694c9 into main Oct 16, 2023
1 check passed
@maciejt10c maciejt10c deleted the code-refactor branch October 16, 2023 07:13
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.

1 participant