Skip to content

Conversation

@hanggrian
Copy link

Description

  • Enforce 80-character column limit.

    This configuration is preferred by Google Shell Style Guide. Also enforces 2-space indentation already defined in EditorConfig.

  • Echo commands without the backslash flag and default value.

    The -e flag can be safely removed because the ASCII code is embedded in the color definitions. Also removes the default value "", printing an empty line is already the expected output of echo.

  • Fix setup.sh by creating the parent folder recursively if the file does not exist.

    writeFileSync may throw an error if the parent directory does not exist. Wrap the call in a function that prepares the parent directory before writing the file.

  • Declare a function if logic is reused at least twice.

    Created functions warn, die, is_installed and is_file.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

Changes Made

  • .editorconfig

  • config.yaml.example

  • core/setup_mcp.sh

  • scripts/generate-env.ts

  • scripts/setup.sh

  • scripts/setup-python.sh → scripts/setup_python.sh

    PEP encourages underscore instead of dash as a separator.

Testing

Describe the tests you ran to verify your changes:

  • Unit tests pass (cd core && pytest tests/)
  • Lint passes (cd core && ruff check .)
  • Manual testing performed

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

- Enforce 2-space indent and 80-character column limit.
- Echo commands without the backslash flag and default value.
- Fix `setup.sh` by creating parent folder recursively if file does not exists.
@hanggrian
Copy link
Author

This change also brings back config.yaml.example removed since #138. We need this file to complete the setup.sh execution, which creates a duplicate file config.yaml.

@hanggrian
Copy link
Author

hanggrian commented Jan 24, 2026

@RichardTang-Aden Apologies, I just found setup-python.sh in the documentation and the CI jobs. There is no particularly good reason to rename this into setup_python.sh, I just thought that it would be a good idea to follow standards and other Python files already in the codebase. Would you rather rename the file back to setup-python.sh or change everything to setup_python.sh.

@RichardTang-Aden
Copy link
Collaborator

Let's use setup-python.sh. We will follow snake_case for all Python files. However, for config, scripts, shell files (such as JSON, TS, and SH) files—we should use hyphens (-).

@RichardTang-Aden
Copy link
Collaborator

RichardTang-Aden commented Jan 24, 2026

Potential Issues Found
1.

[.sh]
max_line_length = 80

The pattern [.sh] only matches a file literally named .sh, not files with .sh extension. It should be [*.sh].

  1. Broken ANSI color codes (Missing escape sequences)
NC='[0m' # No Color
BOLD='[1m'
RED='[91m'
GREEN='[92m'
YELLOW='[93m'

These are missing the escape character (\033 or \e). They should be:

NC=$'\e[0m'
RED=$'\e[91m'
# etc.

Currently, the scripts will print literal [92m text instead of colored output.

  1. Undefined BLUE variable
    `echo "${BLUE}Detected Python:$NC $PYTHON_VERSION"
    The BLUE variable is used but never defined in the color definitions.

  2. Redundant Python version check

if [ "$PYTHON_MINOR" -lt 11 ]; then
  echo "${YELLOW}Warning: Python 3.11+ is recommended..."

This condition can never be true at this point because lines 54-58 already exit if Python < 3.11. This is dead code.

  1. Please also rename the setup_mcp.sh file to setup-mcp.sh Thank you!

@hanggrian
Copy link
Author

  1. EditorConfig file is fixed.

  2. The escape code was there. But honestly, your setup is much better because Git can see changes.

    Screenshot From 2026-01-23 19-58-57

  3. Blue color has been added to quickstart.sh and setup-python.sh.

  4. Removed redundant check in both files.

  5. Configuration file renamed to setup-mcp.sh.

Thanks for the review. I will be more diligent in checking my contribution in the next request.

Copy link
Collaborator

@RichardTang-Aden RichardTang-Aden left a comment

Choose a reason for hiding this comment

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

Tested and the shell worked well for me. Thanks for contributing

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there any reason renaming scripts/setup-python.sh to scripts/setup_python.sh? It seems that setup-python is still used in other places.

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