Skip to content

Add Python build and packaging configuration - #4

Open
mdellison90-stack with Copilot wants to merge 4 commits into
masterfrom
copilot/add-initial-settlement-feature
Open

Add Python build and packaging configuration#4
mdellison90-stack with Copilot wants to merge 4 commits into
masterfrom
copilot/add-initial-settlement-feature

Conversation

Copilot AI commented Jan 25, 2026

Copy link
Copy Markdown

Repository lacked standardized build configuration and dependency management. Added modern Python packaging setup following PEP 518.

Build Configuration

  • requirements.txt - Production dependencies (requests>=2.28.0)
  • requirements-dev.txt - Development tooling (pytest, flake8, black, pylint, mypy)
  • pyproject.toml - Modern build configuration with package discovery and tool settings
  • setup.py - Minimal backwards-compatible shim
  • Makefile - Common tasks: install, install-dev, test, lint, format, clean, docker-build

Infrastructure Updates

  • .gitignore - Python build artifacts, virtual environments, cache files, IDE files
  • Dockerfile - Refactored to use requirements.txt instead of inline pip install
  • README.md - Installation, development, and Docker usage instructions

Package can now be installed via pip install -e . or built as wheel with python -m build.

Original prompt

Pull Request: mdellison90-stack/bbchop#1


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits January 25, 2026 11:36
Co-authored-by: mdellison90-stack <230609064+mdellison90-stack@users.noreply.github.com>
Co-authored-by: mdellison90-stack <230609064+mdellison90-stack@users.noreply.github.com>
Co-authored-by: mdellison90-stack <230609064+mdellison90-stack@users.noreply.github.com>
Copilot AI changed the title [WIP] Add initial settlement feature implementation Add Python build and packaging configuration Jan 25, 2026
@mdellison90-stack
mdellison90-stack marked this pull request as ready for review March 23, 2026 08:52

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1903292974

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread pyproject.toml
Repository = "https://github.com/yasinkuyu/binance-trader"

[tool.setuptools]
packages = ["app"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Fix packaged imports before declaring app installable

Packaging app as an installable wheel here produces a broken install: the modules under app/ still rely on repo-root imports such as import config and from Database import Database, so a wheel built from this commit cannot even import app.Trading outside the source checkout. I verified this by extracting the wheel built from pyproject.toml and importing app.Trading, which fails with ModuleNotFoundError; users taking the new package-install path will get an unusable distribution.

Useful? React with 👍 / 👎.

Comment thread Makefile
docker build -t binance-trader .

docker-run:
docker run binance-trader

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Make docker-run pass the trader's required CLI args

This target always runs docker run binance-trader, but trader.py requires --symbol via argparse before it can start. That means the new make docker-run flow exits immediately for every user, even if the image was built correctly and a valid config is present, so the Makefile now advertises a container run path that is nonfunctional out of the box.

Useful? React with 👍 / 👎.

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